From 2270899e876c82eff9f7649a36d2a3049083a8fc Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 11 Jun 2026 23:56:32 +0200 Subject: [PATCH 1/3] fix(swift-sdk): attribute shielded registration errors to the right step and keep unconfirmed broadcasts safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shielded (Type-20) identity registration ran as one opaque broadcast_and_wait: any failure surfaced as ShieldedBroadcastFailed, the app pinned it on the "Generating Halo 2 proof" step, and the wallet released the spent notes' reservations — even when the transition had already executed on-chain and only the result-proof confirmation failed (observed live on devnet; server side since fixed in #3859). That treated a possibly-live identity as unregistered (orphaned-identity hazard) and invited double-spends. - rs-sdk: add identity_create_from_shielded_pool_transition (build + validate only) so callers can stage broadcast and wait separately. - platform-wallet: classify wait-stage failures; on an ambiguous post-broadcast error, retry fetching the identity by its derived id (success path if found), otherwise return the new ShieldedBroadcastUnconfirmed error and leave note reservations in place (in-memory only; the next nullifier sync reconciles them). - ffi: new result codes ErrorShieldedBroadcastFailed (16) and ErrorShieldedBroadcastUnconfirmed (17); the latter also writes the derived id to out_identity_id. - swift-sdk: mirror the codes; throw a typed ShieldedIdentityCreateUnconfirmedError carrying the derived id. - example app: new .unconfirmed controller phase that holds the identity slot against re-submission and marks the slot used; the shielded progress view gains a distinct "Waiting for platform confirmation" step, attributes broadcast rejections to the broadcast step, and renders an orange confirmation-pending state instead of a red failure. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/error.rs | 16 ++ .../src/shielded_send.rs | 34 +++- packages/rs-platform-wallet/src/error.rs | 16 ++ .../src/wallet/shielded/operations.rs | 144 +++++++++++++- .../identity_create_from_shielded_pool.rs | 56 +++++- .../PlatformWalletManagerShieldedSync.swift | 50 ++++- .../PlatformWallet/PlatformWalletResult.swift | 28 ++- .../IdentityRegistrationController.swift | 78 +++++++- .../Services/RegistrationCoordinator.swift | 20 +- .../Views/CreateIdentityView.swift | 59 +++++- .../Views/PendingRegistrationsList.swift | 16 +- .../Views/RegistrationProgressView.swift | 183 +++++++++++++++--- .../CreateIdentityResumableTests.swift | 9 + 13 files changed, 647 insertions(+), 62 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 8e7e4100220..9a243372713 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -97,6 +97,22 @@ pub enum PlatformWalletFFIResultCode { /// to re-persist. The typed Display rendering still survives as the /// result message for logging/detail. ErrorWalletAlreadyExists = 15, + /// Maps `PlatformWalletError::ShieldedBroadcastFailed`. The shielded + /// identity-create transition was DEFINITIVELY not executed — either the + /// relay/CheckTx rejected the broadcast, or Platform reported the + /// transition's own execution error. The new identity does NOT exist, the + /// spent notes' reservations were released, and the caller is free to + /// retry. `out_identity_id` is left untouched (still zeroed). + ErrorShieldedBroadcastFailed = 16, + /// Maps `PlatformWalletError::ShieldedBroadcastUnconfirmed`. The broadcast + /// was ACCEPTED by the relay but the SDK could not confirm its execution + /// result (a transient result-proof fetch/verify failure, not a platform + /// rejection), and a direct fetch of the derived id also came back empty. + /// The identity may already exist on chain, so the caller must NOT treat + /// it as unregistered or re-submit. UNLIKE every other error code, + /// `out_identity_id` IS written (the 32-byte derived id) on this code so + /// the caller can hold the slot and surface the pending identity. + ErrorShieldedBroadcastUnconfirmed = 17, NotFound = 98, // Used exclusively for all the Option that are retuned as errors ErrorUnknown = 99, diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 503054c5333..90d2252f265 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -51,6 +51,7 @@ use dpp::shielded::{ use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use platform_wallet::wallet::asset_lock::AssetLockFunding; use platform_wallet::wallet::shielded::CachedOrchardProver; +use platform_wallet::PlatformWalletError; use rs_sdk_ffi::{MnemonicResolverCoreSigner, MnemonicResolverHandle, SignerHandle, VTableSigner}; use crate::check_ptr; @@ -467,6 +468,13 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_withdraw( /// `out_identity_id`. The id is deterministic in the spent notes, so the host can also predict it /// independently if needed. /// +/// `out_identity_id` is ALSO written on the [`ErrorShieldedBroadcastUnconfirmed`] result code: the +/// broadcast was accepted but its execution result couldn't be confirmed, so the derived id is +/// handed back (the identity may already exist on chain) and the host must hold the slot rather than +/// treat the registration as failed. On every other error code `out_identity_id` is left untouched. +/// +/// [`ErrorShieldedBroadcastUnconfirmed`]: crate::error::PlatformWalletFFIResultCode::ErrorShieldedBroadcastUnconfirmed +/// /// `send_to_address_on_creation_failure_bytes` is the REQUIRED fallback platform address, supplied /// as raw `PlatformAddress` storage bytes (21 bytes: 1-byte variant tag + 20-byte hash — the /// encoding `PlatformAddress::to_bytes()` produces and `PlatformAddressWasm`/the Swift wrapper @@ -484,7 +492,9 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_withdraw( /// - `signer_identity_handle` must be a valid, non-destroyed `*mut SignerHandle` (a /// `VTableSigner` with the callback variant) that outlives this call; the caller retains /// ownership. -/// - `out_identity_id` must point to 32 writable bytes. +/// - `out_identity_id` must point to 32 writable bytes. It is written on `Success` AND on the +/// `ErrorShieldedBroadcastUnconfirmed` result code (and only those); on all other codes it is +/// left as the caller initialized it. #[no_mangle] #[allow(clippy::too_many_arguments)] pub unsafe extern "C" fn platform_wallet_manager_shielded_identity_create_from_pool( @@ -584,6 +594,28 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_identity_create_from_p *out_identity_id = identity_id.to_buffer(); PlatformWalletFFIResult::ok() } + // Broadcast accepted but its execution result couldn't be confirmed and a direct fetch came + // back empty. The identity MAY exist on chain, so — unlike every other error arm — we still + // write the derived id to `out_identity_id` (see the `# Safety` note) so the caller can hold + // the slot against re-submission and surface the pending identity. The notes' reservations + // were intentionally NOT released wallet-side. + Err(PlatformWalletError::ShieldedBroadcastUnconfirmed { + identity_id, + ref reason, + }) => { + *out_identity_id = identity_id.to_buffer(); + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorShieldedBroadcastUnconfirmed, + format!( + "shielded identity-create-from-pool broadcast unconfirmed (identity {identity_id} may exist on chain): {reason}" + ), + ) + } + // Definitive failure: the transition was not executed and the spent notes were released. + Err(e @ PlatformWalletError::ShieldedBroadcastFailed(_)) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorShieldedBroadcastFailed, + format!("shielded identity-create-from-pool failed: {e}"), + ), Err(e) => PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, format!("shielded identity-create-from-pool failed: {e}"), diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 38bf8ed1795..57c0519e2d5 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -186,6 +186,22 @@ pub enum PlatformWalletError { #[error("Shielded broadcast failed: {0}")] ShieldedBroadcastFailed(String), + /// The shielded identity-create transition was **broadcast and accepted by the relay**, but the + /// SDK could not confirm its execution result (the result-proof fetch/verify failed — e.g. a + /// transient DAPI/proof error, not a platform rejection). The identity with `identity_id` may + /// already exist on chain, so the caller must NOT treat it as unregistered: the slot stays held + /// against re-submission and the spent notes' reservations are left in place (the next nullifier + /// sync reconciles them). `reason` carries the underlying SDK error for diagnostics. + #[error( + "Shielded broadcast succeeded but its execution result could not be confirmed; \ + identity {identity_id} may already exist on chain — do not re-submit \ + (it will appear after the next sync): {reason}" + )] + ShieldedBroadcastUnconfirmed { + identity_id: Identifier, + reason: String, + }, + #[error("Shielded sync failed: {0}")] ShieldedSyncFailed(String), diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index f15a5bf1c3b..f39405d2dee 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -757,22 +757,90 @@ where let identity_id = build.identity_id; - trace!("IdentityCreateFromShieldedPool: built, broadcasting via SDK helper..."); - // Broadcast through the SDK helper, which re-assembles the transition from the PoP-signed - // keys + bundle params (preserving the per-key signatures) and waits for proven execution. - // It returns a `VerifiedIdentityWithShieldedNullifiers` proof result carrying the - // proof-verified `Identity` (and the consumed nullifiers). - let proof_result = sdk - .identity_create_from_shielded_pool( + trace!("IdentityCreateFromShieldedPool: built, broadcasting via SDK..."); + // Stage the broadcast and the result-wait SEPARATELY (instead of one `broadcast_and_wait`) + // so the two failure shapes can be told apart: + // - a broadcast-time rejection (relay/CheckTx refused the tx) means it never executed, so + // releasing the note reservations is correct; + // - a post-broadcast wait failure is AMBIGUOUS — the relay accepted the tx and it may well + // have executed; treating it as "unregistered" + releasing the notes is the + // orphaned-identity + double-spend hazard this split exists to avoid. + // The transition is built once from the PoP-signed keys + bundle params (preserving the + // per-key signatures); the binding signature already committed `identity_id`, the + // denomination, and the full key set. + let st = sdk + .identity_create_from_shielded_pool_transition( build.public_keys, denomination, send_to_address_on_creation_failure, build.bundle, - None, ) + .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; + + // Broadcast (relay-ACK only). A failure here is definitive: the tx was NOT accepted, so the + // spend never happened — map to `ShieldedBroadcastFailed` and let the outer match release + // the reservation, exactly as before. + st.broadcast(sdk, None) .await .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; + // Wait for proven execution. Classify the failure: + // - `StateTransitionBroadcastError` = Platform DEFINITIVELY reported the transition's own + // execution error (it ran and was rejected on its merits). The identity does not exist; + // keep today's behavior (release the reservation via `ShieldedBroadcastFailed`). + // - any other error (DriveProofError / Proof / InvalidProvedResponse / TimeoutReached / + // DapiClientError / …) = AMBIGUOUS: the broadcast was accepted and the transition may + // have executed even though we couldn't fetch/verify its result proof (this is exactly + // the #3859 result-proof incident). Fall back to fetching the identity by its + // pre-derived id before deciding it doesn't exist. + let proof_result = match st + .wait_for_response::(sdk, None) + .await + { + Ok(result) => result, + Err(dash_sdk::Error::StateTransitionBroadcastError(e)) => { + return Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())); + } + Err(wait_err) => { + warn!( + derived_id = %identity_id, + error = %wait_err, + "IdentityCreateFromShieldedPool: broadcast accepted but result confirmation \ + failed; falling back to fetching the identity by its derived id" + ); + // Fetch the identity directly. The id is committed in the sighash and derived + // deterministically from the spent nullifiers, so if the transition landed the row + // is queryable. A freshly-included identity can take a moment to index on the DAPI + // node we hit, so retry a few times with a short, fixed backoff before concluding + // it's truly absent — long enough to ride out routine indexing/replica lag, short + // enough not to wedge the caller's UI for minutes. + match fetch_identity_with_retries(sdk, identity_id).await { + Some(mut identity) => { + info!( + derived_id = %identity_id, + "IdentityCreateFromShieldedPool: result confirmation failed but the \ + identity was found on chain by its derived id; treating as success" + ); + // Same defensive empty-`public_keys` fill as the proven-result path below, + // so downstream auth-key checks see the committed key set immediately. + if identity.public_keys().is_empty() { + identity.set_public_keys(submitted_public_keys.clone()); + } + return Ok::<(Identifier, Identity), PlatformWalletError>(( + identity.id(), + identity, + )); + } + None => { + return Err(PlatformWalletError::ShieldedBroadcastUnconfirmed { + identity_id, + reason: wait_err.to_string(), + }); + } + } + } + }; + // Pull the verified `Identity` out of the proof result. The expected variant is // `VerifiedIdentityWithShieldedNullifiers`; if drive-abci ever returns a different one the // broadcast still SUCCEEDED, so we don't turn it into an error — we synthesize the identity @@ -846,6 +914,15 @@ where ); Ok((identity_id, identity)) } + // The broadcast was accepted but its result couldn't be confirmed and the identity wasn't + // found by a direct fetch. Do NOT `cancel_pending` here: `pending_nullifiers` is in-memory + // only (see `SubwalletState`, "never persisted; the next sync after a crash reconciles"), + // and `mark_spent` during nullifier sync clears matching reservations. So if the transition + // actually executed, the next sync promotes these notes to spent; if it truly never landed, + // an app restart drops the in-memory reservation and frees them. Releasing them now would + // invite double-spend attempts against notes that may already be consumed on chain — the + // very hazard this variant exists to prevent. + Err(e @ PlatformWalletError::ShieldedBroadcastUnconfirmed { .. }) => Err(e), Err(e) => { cancel_pending(store, id, &selected_notes).await; Err(e) @@ -853,6 +930,57 @@ where } } +/// Number of times [`identity_create_from_shielded_pool`] re-fetches the new identity by its +/// derived id after a post-broadcast result-confirmation failure, before declaring the broadcast +/// unconfirmed. +const IDENTITY_CREATE_FETCH_RETRIES: usize = 4; + +/// Fixed backoff between identity fetch attempts. Four attempts ~3 s apart (~9 s of fetch window +/// total) is enough to ride out routine DAPI indexing / replica lag for a freshly-included identity +/// without wedging the caller's UI for minutes. +const IDENTITY_CREATE_FETCH_RETRY_DELAY: std::time::Duration = std::time::Duration::from_secs(3); + +/// Fetch an identity by id with a few fixed-interval retries. +/// +/// Used only on the ambiguous post-broadcast path: the result-proof fetch failed, so we don't know +/// whether the transition executed. The identity id is derived deterministically from the spent +/// notes' nullifiers and committed in the transition sighash, so a successful fetch is positive +/// proof the transition landed. Returns `Some(identity)` on the first hit, or `None` if every +/// attempt comes back empty or errors (transport hiccup, not-yet-indexed, …) — the caller then +/// surfaces `ShieldedBroadcastUnconfirmed` rather than a hard failure. +async fn fetch_identity_with_retries( + sdk: &Arc, + identity_id: Identifier, +) -> Option { + use dash_sdk::platform::Fetch; + + for attempt in 0..IDENTITY_CREATE_FETCH_RETRIES { + match Identity::fetch(sdk, identity_id).await { + Ok(Some(identity)) => return Some(identity), + Ok(None) => { + trace!( + %identity_id, + attempt, + "IdentityCreateFromShieldedPool confirmation fetch: not found yet" + ); + } + Err(e) => { + trace!( + %identity_id, + attempt, + error = %e, + "IdentityCreateFromShieldedPool confirmation fetch errored; will retry" + ); + } + } + // Skip the trailing sleep after the final attempt — nothing follows it. + if attempt + 1 < IDENTITY_CREATE_FETCH_RETRIES { + tokio::time::sleep(IDENTITY_CREATE_FETCH_RETRY_DELAY).await; + } + } + None +} + // ------------------------------------------------------------------------- // Internal helpers (free fns) // ------------------------------------------------------------------------- diff --git a/packages/rs-sdk/src/platform/transition/identity_create_from_shielded_pool.rs b/packages/rs-sdk/src/platform/transition/identity_create_from_shielded_pool.rs index 7586526b088..b78a7d322e9 100644 --- a/packages/rs-sdk/src/platform/transition/identity_create_from_shielded_pool.rs +++ b/packages/rs-sdk/src/platform/transition/identity_create_from_shielded_pool.rs @@ -8,10 +8,34 @@ use dpp::state_transition::proof_result::StateTransitionProofResult; use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::state_transition::state_transitions::shielded::identity_create_from_shielded_pool_transition::methods::IdentityCreateFromShieldedPoolTransitionMethodsV0; use dpp::state_transition::state_transitions::shielded::identity_create_from_shielded_pool_transition::IdentityCreateFromShieldedPoolTransition; +use dpp::state_transition::StateTransition; /// Helper trait to create a brand-new Platform identity funded directly from the shielded pool. #[async_trait::async_trait] pub trait IdentityCreateFromShieldedPool { + /// Build (and structurally validate) the Type-20 transition from PoP-signed keys + bundle + /// params, **without** broadcasting it. + /// + /// Splitting the build off `identity_create_from_shielded_pool` lets a caller stage the + /// broadcast and the result-wait separately (via [`BroadcastStateTransition::broadcast`] then + /// [`BroadcastStateTransition::wait_for_response`]). That separation is what lets the wallet + /// distinguish a *broadcast-time* rejection (notes safe to release) from a *post-broadcast* + /// confirmation failure where the transition may well have executed on chain (notes must stay + /// reserved) — a single `broadcast_and_wait` `Result` collapses the two and misattributes the + /// latter, which is exactly the orphaned-identity hazard this split fixes. + /// + /// `public_keys` MUST already carry their per-key proof-of-possession signatures over the + /// transition's signable bytes (the wallet/builder fills them before broadcast); the bundle's + /// binding signature commits the derived id + denomination + full key set. No platform identity + /// signature is involved. + fn identity_create_from_shielded_pool_transition( + &self, + public_keys: Vec, + denomination: u64, + send_to_address_on_creation_failure: PlatformAddress, + bundle: OrchardBundleParams, + ) -> Result; + /// Create a new identity funded by spending shielded-pool notes. /// /// The exit amount is a fixed `denomination` (a member of the versioned denomination set), and @@ -40,14 +64,13 @@ pub trait IdentityCreateFromShieldedPool { #[async_trait::async_trait] impl IdentityCreateFromShieldedPool for Sdk { - async fn identity_create_from_shielded_pool( + fn identity_create_from_shielded_pool_transition( &self, public_keys: Vec, denomination: u64, send_to_address_on_creation_failure: PlatformAddress, bundle: OrchardBundleParams, - settings: Option, - ) -> Result { + ) -> Result { let OrchardBundleParams { actions, anchor, @@ -67,9 +90,30 @@ impl IdentityCreateFromShieldedPool for Sdk { )?; ensure_valid_state_transition_structure(&state_transition, self.version())?; - // Wait for proven inclusion (parity with `unshield`/`shielded_transfer`/`withdraw`), so the - // wallet's post-broadcast `finalize_pending` only runs once the spend is proven — a - // Platform-level rejection after relay then correctly triggers the `cancel_pending` fallback. + Ok(state_transition) + } + + async fn identity_create_from_shielded_pool( + &self, + public_keys: Vec, + denomination: u64, + send_to_address_on_creation_failure: PlatformAddress, + bundle: OrchardBundleParams, + settings: Option, + ) -> Result { + // Build + structurally validate via the shared builder, then wait for proven inclusion + // (parity with `unshield`/`shielded_transfer`/`withdraw`), so the wallet's post-broadcast + // `finalize_pending` only runs once the spend is proven — a Platform-level rejection after + // relay then correctly triggers the `cancel_pending` fallback. Callers that need to + // distinguish broadcast-time from post-broadcast failures should instead drive the two + // `BroadcastStateTransition` stages themselves off the transition returned by + // `identity_create_from_shielded_pool_transition`. + let state_transition = self.identity_create_from_shielded_pool_transition( + public_keys, + denomination, + send_to_address_on_creation_failure, + bundle, + )?; let proof_result = state_transition .broadcast_and_wait::(self, settings) .await?; diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift index 8e9302b2e75..8fb35da1f5c 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift @@ -1,6 +1,30 @@ import Foundation import DashSDKFFI +/// Thrown by `shieldedIdentityCreateFromPool` when the Type-20 +/// transition was broadcast and ACCEPTED by the relay, but the SDK +/// could not confirm its execution result and a direct fetch of the +/// derived id also came back empty (the Rust side already retried). +/// +/// This is NOT a registration failure: the identity may already exist +/// on chain (the broadcast landed; only the result-proof confirmation +/// failed — e.g. a transient DAPI/proof error). The caller MUST hold +/// the slot against re-submission and surface the pending identity +/// rather than treating it as unregistered — re-registering the same +/// keys while the identity is live would fail the registered-key-hash +/// stateful check and burn the funds. The note reservations were +/// intentionally left in place wallet-side; the next nullifier sync +/// reconciles them. +/// +/// `identityId` is the 32-byte derived id the FFI filled into +/// `outIdentityId` on this specific result code (valid, deterministic +/// in the spent notes). `message` is the Rust-supplied diagnostic. +public struct ShieldedIdentityCreateUnconfirmedError: LocalizedError { + public let identityId: Data + public let message: String + public var errorDescription: String? { message } +} + /// Per-wallet outcome from a completed shielded sync pass. /// /// Mirrors the Rust-side @@ -787,8 +811,30 @@ extension PlatformWalletManager { } } - try result.check() - return withUnsafeBytes(of: outIdentityId) { Data($0) } + // Wrap the FFI result EXACTLY ONCE so its Rust-owned message is + // freed once in `deinit` (don't also call `result.check()`, which + // would construct a second wrapper over the same struct and + // double-free). Inspect the typed code directly: + // - success: return the derived id. + // - unconfirmed: the broadcast landed but its result couldn't be + // confirmed; Rust filled `outIdentityId` with the derived id on + // THIS code (and only this code). Throw the typed + // `ShieldedIdentityCreateUnconfirmedError` so the caller holds + // the slot instead of treating it as failed. + // - any other non-success: throw the regular typed error. + let wrapped = PlatformWalletResult(result) + switch wrapped.code { + case .success: + return withUnsafeBytes(of: outIdentityId) { Data($0) } + case .errorShieldedBroadcastUnconfirmed: + let identityId = withUnsafeBytes(of: outIdentityId) { Data($0) } + throw ShieldedIdentityCreateUnconfirmedError( + identityId: identityId, + message: wrapped.message ?? "shielded identity-create broadcast unconfirmed" + ) + default: + throw PlatformWalletError(result: wrapped) + } }.value } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index 942ef996dd4..04d15e31d15 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -21,6 +21,15 @@ public enum PlatformWalletResultCode: Int32, Sendable { case errorArithmeticOverflow = 13 case errorNoSelectableInputs = 14 case errorWalletAlreadyExists = 15 + /// Definitive shielded-broadcast failure: the Type-20 transition was not + /// executed (relay/CheckTx rejection or a platform-reported execution + /// error), the spent notes were released, and the caller may retry. + case errorShieldedBroadcastFailed = 16 + /// Shielded broadcast accepted but its execution result could not be + /// confirmed; the identity may already exist on chain. The FFI also fills + /// the derived id into `outIdentityId` on this code, so the caller can + /// hold the slot rather than treat the registration as failed. + case errorShieldedBroadcastUnconfirmed = 17 case notFound = 98 case errorUnknown = 99 @@ -58,6 +67,10 @@ public enum PlatformWalletResultCode: Int32, Sendable { self = .errorNoSelectableInputs case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_WALLET_ALREADY_EXISTS: self = .errorWalletAlreadyExists + case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_SHIELDED_BROADCAST_FAILED: + self = .errorShieldedBroadcastFailed + case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_SHIELDED_BROADCAST_UNCONFIRMED: + self = .errorShieldedBroadcastUnconfirmed case PLATFORM_WALLET_FFI_RESULT_CODE_NOT_FOUND: self = .notFound case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_UNKNOWN: @@ -136,6 +149,16 @@ public enum PlatformWalletError: LocalizedError { case arithmeticOverflow(String) case noSelectableInputs(String) case walletAlreadyExists(String) + /// Definitive shielded-broadcast failure: the Type-20 transition was not + /// executed and the spent notes were released; safe to retry. + case shieldedBroadcastFailed(String) + /// Shielded broadcast accepted but its execution result could not be + /// confirmed; the identity may already exist on chain. Callers that need + /// the derived id should special-case the + /// `.errorShieldedBroadcastUnconfirmed` result code (which carries + /// `outIdentityId`) before falling back to this error — see + /// `ShieldedIdentityCreateUnconfirmedError`. + case shieldedBroadcastUnconfirmed(String) case notFound(String) case unknown(String) @@ -149,7 +172,8 @@ public enum PlatformWalletError: LocalizedError { .identityNotFound(let m), .contactNotFound(let m), .utf8Conversion(let m), .serialization(let m), .deserialization(let m), .memoryAllocation(let m), .arithmeticOverflow(let m), .noSelectableInputs(let m), - .walletAlreadyExists(let m), + .walletAlreadyExists(let m), .shieldedBroadcastFailed(let m), + .shieldedBroadcastUnconfirmed(let m), .notFound(let m), .unknown(let m): return m } @@ -177,6 +201,8 @@ public enum PlatformWalletError: LocalizedError { case .errorArithmeticOverflow: self = .arithmeticOverflow(detail) case .errorNoSelectableInputs: self = .noSelectableInputs(detail) case .errorWalletAlreadyExists: self = .walletAlreadyExists(detail) + case .errorShieldedBroadcastFailed: self = .shieldedBroadcastFailed(detail) + case .errorShieldedBroadcastUnconfirmed: self = .shieldedBroadcastUnconfirmed(detail) case .notFound: self = .notFound(detail) case .errorUnknown: self = .unknown(detail) } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift index cd457c89b8f..3d36f658514 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift @@ -53,6 +53,22 @@ final class IdentityRegistrationController: ObservableObject { /// the coordinator's map until the user dismisses it /// manually. case failed(String) + /// Broadcast-succeeded-but-confirmation-failed terminal state + /// (shielded Type-20 only). The transition was relayed and + /// accepted, but its execution-result proof couldn't be + /// confirmed and a direct fetch of the derived id also came + /// back empty (Rust already retried). The identity at + /// `identityId` MAY already be live on chain. + /// + /// This is NOT a red failure: treating it as unregistered is + /// the orphaned-identity hazard. Re-registering the same keys + /// while the identity is live would fail the + /// registered-key-hash stateful check and burn the funds to + /// the fallback address minus a penalty — so the slot must stay + /// held (see `isActive`) and the entry must linger in Pending + /// Registrations until the identity row appears via the next + /// sync and the user dismisses it. + case unconfirmed(identityId: Data, message: String) /// Whether the controller is currently holding its slot /// against a fresh registration. Used by the Resumable @@ -69,9 +85,12 @@ final class IdentityRegistrationController: ObservableObject { /// which the identity-slot anti-join already covers. /// `.failed` is NOT active either: the user is expected /// to retry, so the lock should resurface for selection. + /// `.unconfirmed` IS active: the identity is probably live on + /// chain, so the slot must stay held to block a re-submission + /// that would burn funds against the registered-key-hash check. var isActive: Bool { switch self { - case .preparingKeys, .inFlight: + case .preparingKeys, .inFlight, .unconfirmed: return true case .idle, .completed, .failed: return false @@ -79,11 +98,33 @@ final class IdentityRegistrationController: ObservableObject { } } + /// Which stage a shielded `.failed` terminal state failed at, so + /// `RegistrationProgressView` can attribute the red marker to the + /// right step instead of always blaming the Halo 2 proof step. + /// Only meaningful for `fundingKind == .shieldedPool` and only set + /// alongside a `.failed` phase. + enum FailureStage { + /// Failed before or during the broadcast itself — build / proof + /// error, or a relay/CheckTx broadcast rejection. The shielded + /// progress view keeps the existing elapsed-time heuristic + /// (note-selection vs Halo 2 proof) for the pre-broadcast slice. + case beforeBroadcast + /// Platform definitively rejected the broadcast transition (a + /// `PlatformWalletError.shieldedBroadcastFailed`). Attributed to + /// the "Broadcasting transition" step. + case broadcastRejected + } + /// Current phase. Updates flow: /// `.idle` → `.preparingKeys` (caller) → `.inFlight` (submit) → - /// `.completed(id) | .failed(message)`. + /// `.completed(id) | .failed(message) | .unconfirmed(id, message)`. @Published private(set) var phase: Phase = .idle + /// Stage attribution for a shielded `.failed` phase. `nil` whenever + /// the phase is not a shielded failure. Reset at the start of every + /// `submit` so a retry doesn't inherit the previous attempt's stage. + @Published private(set) var failureStage: FailureStage? + /// Slot this controller is bound to. Stored so the coordinator /// and the progress view can filter `PersistentAssetLock` rows /// by `(walletId, identityIndex)`. @@ -132,6 +173,9 @@ final class IdentityRegistrationController: ObservableObject { /// - `.completed`: re-submitting after success would flip the /// UI from "Done" back to a spinner before failing on the /// consumed lock. + /// - `.unconfirmed`: the identity is probably already live on + /// chain; re-submitting the same keys would fail the + /// registered-key-hash stateful check and burn the funds. /// `.idle`, `.preparingKeys`, and `.failed` are allowed — the /// coordinator drives the legitimate-restart flow through them /// (callers must call `enterPreparingKeys()` before `submit()`, @@ -140,15 +184,19 @@ final class IdentityRegistrationController: ObservableObject { /// `body` performs the actual FFI call. It runs detached on a /// background priority and reports the identity id on success /// or rethrows on failure. The controller flips `phase` to - /// `.completed` / `.failed` accordingly. + /// `.completed` / `.unconfirmed` / `.failed` accordingly, and on a + /// shielded `.failed` records `failureStage` for step attribution. func submit(body: @escaping () async throws -> Data) { switch phase { case .idle, .preparingKeys, .failed: break - case .inFlight, .completed: + case .inFlight, .completed, .unconfirmed: return } phase = .inFlight + // Clear any stage attribution carried over from a previous failed + // attempt before this one runs. + failureStage = nil lastSubmittedAt = Date() task = Task { [weak self] in do { @@ -156,8 +204,30 @@ final class IdentityRegistrationController: ObservableObject { await MainActor.run { self?.phase = .completed(identityId: identityId) } + } catch let unconfirmed as ShieldedIdentityCreateUnconfirmedError { + // Broadcast landed but its result couldn't be confirmed and + // Rust's direct fetch came back empty. Hold the slot; the + // identity probably exists and will surface on the next sync. + await MainActor.run { + self?.phase = .unconfirmed( + identityId: unconfirmed.identityId, + message: unconfirmed.message + ) + } } catch { + // Attribute the failure stage so the shielded progress view + // can point the red marker at the right step. A + // `shieldedBroadcastFailed` is a definitive platform/relay + // rejection of the broadcast; everything else (build / Halo 2 + // proof errors) failed before the broadcast. + let stage: FailureStage + if case PlatformWalletError.shieldedBroadcastFailed = error { + stage = .broadcastRejected + } else { + stage = .beforeBroadcast + } await MainActor.run { + self?.failureStage = stage self?.phase = .failed(error.localizedDescription) } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift index ec1cff65649..e3fcba18dc9 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift @@ -96,10 +96,13 @@ final class RegistrationCoordinator: ObservableObject { let key = SlotKey(walletId: walletId, identityIndex: identityIndex) if let existing = controllers[key] { switch existing.phase { - case .preparingKeys, .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. + case .preparingKeys, .inFlight, .completed, .unconfirmed: + // Active, just-completed, or unconfirmed — don't re-enter. + // Returning the existing controller lets the caller bind to + // its progress / terminal state without disrupting it. For + // `.unconfirmed` in particular, re-submitting would race a + // duplicate registration against an identity that's probably + // already live on chain. return existing case .idle, .failed: // Legitimate restart paths: a brand-new idle @@ -166,10 +169,11 @@ final class RegistrationCoordinator: ObservableObject { } return } - case .failed: - // Keep indefinitely; the user dismisses manually - // via the "Dismiss" action. Return so the poll - // loop doesn't spin. + case .failed, .unconfirmed: + // Keep indefinitely; the user dismisses manually via the + // "Dismiss" action (for `.unconfirmed`, only after the + // identity row appears via sync). Return so the poll loop + // doesn't spin. return default: completedAt = nil diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift index aec3cd8de9a..dd1f4619210 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift @@ -423,6 +423,46 @@ struct CreateIdentityView: View { .textSelection(.enabled) } } + case .unconfirmed(let identityId, let message): + Section { + VStack(alignment: .leading, spacing: 8) { + Label( + "Broadcast succeeded — confirmation pending", + systemImage: "exclamationmark.triangle.fill" + ) + .foregroundColor(.orange) + .font(.headline) + Text(identityId.toBase58String()) + .font(.system(.caption, design: .monospaced)) + .foregroundColor(.secondary) + .textSelection(.enabled) + Text(message) + .font(.callout) + .foregroundColor(.primary) + .textSelection(.enabled) + Text( + "The transition was broadcast and accepted, but its " + + "execution-result proof couldn't be confirmed. The " + + "identity will appear in the Identities tab after the " + + "next sync. Do NOT re-register these keys — the slot " + + "is held to prevent burning funds." + ) + .font(.caption) + .foregroundColor(.secondary) + // Plain sheet dismiss only — leave the controller on the + // coordinator so the entry stays in Pending Registrations + // until the identity row appears via sync and the user + // dismisses it there. + Button { + dismiss() + } label: { + Text("Close") + .frame(maxWidth: .infinity) + } + .buttonStyle(.bordered) + .padding(.top, 4) + } + } default: EmptyView() } @@ -1422,6 +1462,23 @@ struct CreateIdentityView: View { // controller so the destination can render // the inline failure state. return + case .unconfirmed: + // Broadcast landed but its result couldn't be + // confirmed; the identity is probably already live on + // chain. Mark the slot used (same as `.completed`) so + // the next registration can't reuse these keys and + // burn funds against the registered-key-hash stateful + // check. We do NOT persist a `PersistentIdentity` row + // here — the proof-verified identity wasn't returned; + // the next sync writes the row once the identity is + // confirmed on chain. + markIdentitySlotUsed( + walletId: walletId, + identityIndex: identityIndex + ) + try? modelContext.save() + self.isCreating = false + return default: continue } @@ -1448,7 +1505,7 @@ struct CreateIdentityView: View { lastEmitted = phase } switch phase { - case .completed, .failed: + case .completed, .failed, .unconfirmed: continuation.finish() return default: diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift index e1f60d47bca..3ccb6880e15 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift @@ -72,7 +72,11 @@ struct PendingRegistrationRow: View { .padding(.vertical, 2) } .swipeActions(edge: .trailing, allowsFullSwipe: false) { - if case .failed = controller.phase { + // Both terminal-but-retained states are user-dismissable. + // `.unconfirmed` stays until the user clears it (typically once + // the identity row appears via sync); dismissing it only drops + // the Pending row, it doesn't undo the on-chain registration. + if isDismissable { Button { walletManager.registrationCoordinator.dismiss( walletId: controller.walletId, @@ -86,11 +90,19 @@ struct PendingRegistrationRow: View { } } + private var isDismissable: Bool { + switch controller.phase { + case .failed, .unconfirmed: return true + default: return false + } + } + private var phaseIcon: String { switch controller.phase { case .idle, .preparingKeys, .inFlight: return "clock.fill" case .completed: return "checkmark.circle.fill" case .failed: return "xmark.octagon.fill" + case .unconfirmed: return "exclamationmark.triangle.fill" } } @@ -99,6 +111,7 @@ struct PendingRegistrationRow: View { case .idle, .preparingKeys, .inFlight: return .blue case .completed: return .green case .failed: return .red + case .unconfirmed: return .orange } } @@ -109,6 +122,7 @@ struct PendingRegistrationRow: View { case .inFlight: return "In flight" case .completed: return "Registered" case .failed: return "Failed" + case .unconfirmed: return "Confirmation pending" } } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift index ec1f1a2750f..2ed785741da 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift @@ -28,20 +28,30 @@ import SwiftDashSDK /// 5. Registering identity → activeLock `statusRaw == 2 or 3` /// AND controller still `.inFlight` /// -/// Shielded-pool funding (4 steps, phase + elapsed-time driven — there +/// Shielded-pool funding (5 steps, phase + elapsed-time driven — there /// is no asset lock and no per-stage signal from Rust during the opaque /// FFI call; see `shieldedCurrentStep`): /// -/// 1. Selecting shielded notes 2. Generating Halo 2 proof -/// 3. Broadcasting transition 4. Registering identity +/// 1. Selecting shielded notes 2. Generating Halo 2 proof +/// 3. Broadcasting transition 4. Waiting for platform confirmation +/// 5. Registering identity +/// +/// Step 4 ("Waiting for platform confirmation") exists so a +/// post-broadcast confirmation failure is attributed there — NOT to the +/// Halo 2 proof step — and so the `.unconfirmed` terminal state (the +/// broadcast landed but its result couldn't be confirmed) has a step to +/// render an orange warning on while steps 1–3 stay done and step 5 +/// stays pending. /// /// `.completed` is the *terminal* state and is not a separate step; /// `RegistrationProgressView` renders the "Identity created" banner /// + "View Identity" navigation below this section in its own /// terminalSection. `.failed` marks the current step with the error -/// icon + message. Step 4 is shown as `.skipped` (faded checkmark) -/// when the IS branch came back fast so the user can see step 4 was -/// passed through without engaging the CL fallback. +/// icon + message. `.unconfirmed` marks step 4 with an orange warning +/// triangle and renders its own terminalSection banner. (Asset-lock) +/// step 4 is shown as `.skipped` (faded checkmark) when the IS branch +/// came back fast so the user can see step 4 was passed through without +/// engaging the CL fallback. struct RegistrationProgressSection: View { @ObservedObject var controller: IdentityRegistrationController @@ -79,12 +89,12 @@ struct RegistrationProgressSection: View { /// Number of visual steps for the active funding source. The /// asset-lock path has 5 (build → broadcast → IS → CL → register); - /// the shielded-pool path has 4 (select notes → Halo 2 proof → - /// broadcast → register). + /// the shielded-pool path has 5 (select notes → Halo 2 proof → + /// broadcast → wait for confirmation → register). private var stepCount: Int { switch controller.fundingKind { case .assetLock: return 5 - case .shieldedPool: return 4 + case .shieldedPool: return 5 } } @@ -99,6 +109,7 @@ struct RegistrationProgressSection: View { let count = stepCount let step = currentStep(now: now) let isFailed = isFailed + let isUnconfirmed = isUnconfirmed let errorMessage = failureMessage Section { @@ -106,9 +117,17 @@ struct RegistrationProgressSection: View { stepRow( index: idx, title: stepTitle(idx), - state: stepState(idx, currentStep: step, isFailed: isFailed) + state: stepState( + idx, + currentStep: step, + isFailed: isFailed, + isUnconfirmed: isUnconfirmed + ) ) - if idx == count, let message = errorMessage { + // Only `.failed` renders an inline red message under the + // last step; `.unconfirmed` gets its own orange + // terminalSection banner instead. + if idx == count, isFailed, let message = errorMessage { Text(message) .font(.caption) .foregroundColor(.red) @@ -118,7 +137,7 @@ struct RegistrationProgressSection: View { } header: { Text("Registration Progress") } footer: { - Text(footerText(step: step, isFailed: isFailed)) + Text(footerText(step: step, isFailed: isFailed, isUnconfirmed: isUnconfirmed)) .font(.caption2) .foregroundColor(.secondary) } @@ -161,6 +180,12 @@ struct RegistrationProgressSection: View { } } return 5 + case .unconfirmed: + // `.unconfirmed` is a shielded-only terminal state and is + // handled in `shieldedCurrentStep`; it never reaches this + // asset-lock branch. Report the last (register) step so the + // switch is exhaustive and the rows render fully done. + return 5 case .inFlight: guard let lock = activeLocks.first else { // No lock row yet — Rust has the slot but hasn't @@ -210,35 +235,49 @@ struct RegistrationProgressSection: View { /// this just lets the user see step 1 register before step 2 spins. private static let shieldedNoteSelectionWindow: TimeInterval = 2.0 - /// Step 1...4 for the shielded-pool funding path. There is NO + /// Step 1...5 for the shielded-pool funding path. There is NO /// per-stage signal from Rust during the opaque /// `platform_wallet_manager_shielded_identity_create_from_pool` - /// call (note-select → Halo 2 proof → broadcast → confirm all run - /// inside one blocking FFI call), so transitions are driven from - /// `controller.phase` plus elapsed time since `lastSubmittedAt`: + /// call (note-select → Halo 2 proof → broadcast → confirm → register + /// all run inside one blocking FFI call), so transitions are driven + /// from `controller.phase` plus elapsed time since `lastSubmittedAt`: /// /// 1. Selecting shielded notes → `.idle` / `.preparingKeys`, or /// the first `shieldedNoteSelectionWindow` seconds of `.inFlight`. /// 2. Generating Halo 2 proof → `.inFlight` after that window. - /// Kept active for the rest of the call: broadcast + confirm - /// (steps 3/4) can't be observed separately, so they stay - /// `.pending` rather than flipping to a green check for work - /// that may not have happened yet. - /// On `.completed` return 5 (one past the last step) so all rows - /// render `.done`. On `.failed` mark the step we'd reached. + /// Kept active for the rest of the call: broadcast / confirm / + /// register (steps 3/4/5) can't be observed separately while the + /// single FFI call is in flight, so they stay `.pending` rather + /// than flipping to a green check for work that may not have + /// happened yet. + /// On `.completed` return 6 (one past the last step) so all rows + /// render `.done`. On `.unconfirmed` return 4 ("Waiting for platform + /// confirmation") so that step carries the warning. On `.failed` + /// attribute the step: a `broadcastRejected` failure marks step 3 + /// ("Broadcasting transition"); anything else keeps the + /// note-selection vs Halo 2 elapsed-time heuristic. private func shieldedCurrentStep(now: Date) -> Int { switch controller.phase { case .idle, .preparingKeys: return 1 case .completed: - return 5 + return 6 + case .unconfirmed: + // Broadcast landed; only the result confirmation failed. + // Attribute to the "Waiting for platform confirmation" step. + return 4 case .inFlight: guard let submittedAt = controller.lastSubmittedAt else { return 1 } let elapsed = now.timeIntervalSince(submittedAt) return elapsed < Self.shieldedNoteSelectionWindow ? 1 : 2 case .failed: - // Fail on the proof step unless we never left note - // selection (no submit timestamp yet). + // A definitive broadcast rejection is attributed to the + // broadcast step (3). Build / Halo 2 proof errors fail before + // the broadcast, so keep the elapsed-time heuristic + // (note-selection vs proof) for them. + if controller.failureStage == .broadcastRejected { + return 3 + } guard let submittedAt = controller.lastSubmittedAt else { return 1 } let elapsed = now.timeIntervalSince(submittedAt) return elapsed < Self.shieldedNoteSelectionWindow ? 1 : 2 @@ -292,6 +331,14 @@ struct RegistrationProgressSection: View { return false } + /// True only for the shielded `.unconfirmed` terminal state. Drives + /// the orange warning on step 4 (and keeps it distinct from + /// `isFailed`, which `.unconfirmed` is NOT). + private var isUnconfirmed: Bool { + if case .unconfirmed = controller.phase { return true } + return false + } + private var failureMessage: String? { if case .failed(let msg) = controller.phase { return msg } return nil @@ -303,7 +350,8 @@ struct RegistrationProgressSection: View { case 1: return "Selecting shielded notes" case 2: return "Generating Halo 2 proof" case 3: return "Broadcasting transition" - case 4: return "Registering identity" + case 4: return "Waiting for platform confirmation" + case 5: return "Registering identity" default: return "" } } @@ -322,12 +370,28 @@ struct RegistrationProgressSection: View { /// the IS branch returned the proof without needing ChainLock /// fallback — visually distinguishable so users don't think /// the step "didn't happen yet" once we've moved past it. - enum StepState { case done, active, pending, skipped, failed } + /// `.warning` (orange triangle) marks the "Waiting for platform + /// confirmation" step when the broadcast landed but its result + /// couldn't be confirmed (the shielded `.unconfirmed` terminal + /// state) — distinct from `.failed` because nothing is actually + /// wrong; the identity is probably live on chain. + enum StepState { case done, active, pending, skipped, failed, warning } - private func stepState(_ idx: Int, currentStep: Int, isFailed: Bool) -> StepState { + private func stepState( + _ idx: Int, + currentStep: Int, + isFailed: Bool, + isUnconfirmed: Bool + ) -> StepState { if isFailed && idx == currentStep { return .failed } + // `.unconfirmed` warns on its current step (step 4) while leaving + // the earlier steps `.done` (`idx < currentStep` below) and the + // register step `.pending` (`idx > currentStep`). + if isUnconfirmed && idx == currentStep { + return .warning + } if idx < currentStep { // Steps 3 and 4 are the IS / CL halves of the proof // round: exactly one of them is skipped on a successful @@ -403,6 +467,12 @@ struct RegistrationProgressSection: View { Image(systemName: "xmark.octagon.fill") .foregroundColor(.red) .font(.title3) + case .warning: + // Broadcast landed but its result couldn't be confirmed — + // not an error, so an orange triangle, not the red octagon. + Image(systemName: "exclamationmark.triangle.fill") + .foregroundColor(.orange) + .font(.title3) } } @@ -413,10 +483,17 @@ struct RegistrationProgressSection: View { case .pending: return .secondary case .skipped: return .secondary case .failed: return .red + case .warning: return .orange } } - private func footerText(step: Int, isFailed: Bool) -> String { + private func footerText(step: Int, isFailed: Bool, isUnconfirmed: Bool) -> String { + if isUnconfirmed { + return "The transition was broadcast, but confirmation of its " + + "result proof failed. The identity may already exist on " + + "chain and will appear after the next sync — do not " + + "re-submit." + } if isFailed { return "Tap Dismiss in Pending Registrations to clear this entry." } @@ -425,7 +502,8 @@ struct RegistrationProgressSection: View { case 1: return "Selecting shielded notes to spend from the pool." case 2: return "Generating the Halo 2 proof — this can take ~1–2 minutes." case 3: return "Broadcasting the IdentityCreateFromShieldedPool transition." - case 4: return "Registering the proof-verified identity on Platform." + case 4: return "Waiting for Platform to confirm the transition's execution result." + case 5: return "Registering the proof-verified identity on Platform." default: return "" } } @@ -510,6 +588,51 @@ struct RegistrationProgressView: View { .textSelection(.enabled) } } + case .unconfirmed(let identityId, let message): + Section { + VStack(alignment: .leading, spacing: 8) { + Label( + "Broadcast succeeded — confirmation pending", + systemImage: "exclamationmark.triangle.fill" + ) + .foregroundColor(.orange) + .font(.headline) + // Same base58 styling as the completed case — the id is + // the derived identity id Rust handed back; the identity + // is probably already live on chain. + Text(identityId.toBase58String()) + .font(.system(.caption, design: .monospaced)) + .foregroundColor(.secondary) + .textSelection(.enabled) + Text(message) + .font(.callout) + .foregroundColor(.primary) + .textSelection(.enabled) + Text( + "The transition was broadcast and accepted, but its " + + "execution-result proof couldn't be confirmed. The " + + "identity above will appear in the Identities tab " + + "after the next sync. Do NOT re-submit — the slot is " + + "held to prevent burning funds against a duplicate " + + "registration." + ) + .font(.caption) + .foregroundColor(.secondary) + // Plain navigation dismiss only — it must NOT drop the + // controller from the coordinator (that frees the slot). + // The entry stays in Pending Registrations until the + // identity row appears via sync and the user dismisses it + // there. + Button { + dismiss() + } label: { + Text("Close") + .frame(maxWidth: .infinity) + } + .buttonStyle(.bordered) + .padding(.top, 4) + } + } default: EmptyView() } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift index 1bbb2b1a852..c17cad58eb5 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift @@ -293,5 +293,14 @@ final class CreateIdentityResumableTests: XCTestCase { ) XCTAssertFalse(IdentityRegistrationController.Phase.failed("nope").isActive, ".failed: user is expected to retry — let the lock resurface") + XCTAssertTrue( + IdentityRegistrationController.Phase + .unconfirmed( + identityId: Data(repeating: 0xDD, count: 32), + message: "pending" + ) + .isActive, + ".unconfirmed: identity is probably live on chain — keep the slot held so a re-submission can't burn funds against the registered-key-hash check" + ) } } From 420628a0dbf134ee17698bd2ede72ebd0e440913 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:20:58 +0200 Subject: [PATCH 2/3] fix(swift-sdk): harden review findings on shielded unconfirmed-broadcast handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses thepastaclaw review on #3862: - ffi: map ShieldedBroadcastFailed/Unconfirmed to codes 16/17 in the blanket From impl so future entry points that propagate via `?` can't flatten them to ErrorUnknown (the dedicated identity-create match still owns the out_identity_id write); pinned by a new unit test. - platform-wallet: extract wait_error_is_definitive_rejection so the wait-stage classification (definitive rejection vs ambiguous confirmation failure) is pinned by unit tests. - example app: drop FailureStage.beforeBroadcast — only a typed shieldedBroadcastFailed is confidently attributable; everything else stays unattributed (nil) and the progress view falls back to its elapsed-time heuristic instead of blaming the Halo 2 step by elimination. - example app: gate dismissing an .unconfirmed Pending Registrations row on the slot's PersistentIdentity row existing — while unconfirmed-and-unpersisted the live controller is the only guard keeping the identity index un-selectable (usedIdentityIndices ignores the deprecated isUsed flag), so an early dismiss could free the slot for a fund-burning duplicate registration. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/error.rs | 55 +++++++++++++++ .../src/wallet/shielded/operations.rs | 67 ++++++++++++++++++- .../IdentityRegistrationController.swift | 52 ++++++++------ .../Views/CreateIdentityView.swift | 25 ++++--- .../Views/PendingRegistrationsList.swift | 53 +++++++++++++-- .../Views/RegistrationProgressView.swift | 30 +++++---- 6 files changed, 232 insertions(+), 50 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 9a243372713..cd7b7d9f776 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -207,6 +207,22 @@ impl From for PlatformWalletFFIResult { PlatformWalletError::WalletAlreadyExists(..) => { PlatformWalletFFIResultCode::ErrorWalletAlreadyExists } + // The two shielded broadcast/wait variants. Today nothing routes + // them through this blanket impl — the dedicated match in + // `platform_wallet_manager_shielded_identity_create_from_pool` + // (`shielded_send.rs`) owns them so it can also write + // `out_identity_id` on the unconfirmed code. But any *future* FFI + // entry point that propagates these via `?` / `.into()` would + // otherwise silently flatten them to `ErrorUnknown` and defeat the + // slot-holding contract. A blanket conversion can't write + // `out_identity_id` (it has no out-param), so the most it can do is + // keep the typed code alive — which is what these arms guarantee. + PlatformWalletError::ShieldedBroadcastFailed(..) => { + PlatformWalletFFIResultCode::ErrorShieldedBroadcastFailed + } + PlatformWalletError::ShieldedBroadcastUnconfirmed { .. } => { + PlatformWalletFFIResultCode::ErrorShieldedBroadcastUnconfirmed + } _ => PlatformWalletFFIResultCode::ErrorUnknown, }; PlatformWalletFFIResult::err(code, error.to_string()) @@ -510,6 +526,45 @@ mod tests { ); } + /// The two shielded broadcast/wait variants map to their dedicated FFI + /// codes through the blanket `From` impl rather than flattening to + /// `ErrorUnknown`. The dedicated `shielded_send.rs` match owns the live + /// path (it also writes `out_identity_id` on the unconfirmed code), but + /// any future entry point propagating these via `?` / `.into()` must keep + /// the typed code — these arms guarantee that. The typed Display rendering + /// still survives as the message. + #[test] + fn shielded_broadcast_variants_map_to_dedicated_codes() { + let failed = PlatformWalletError::ShieldedBroadcastFailed("relay rejected".to_string()); + let rendered = failed.to_string(); + let result: PlatformWalletFFIResult = failed.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorShieldedBroadcastFailed, + "ShieldedBroadcastFailed should map to ErrorShieldedBroadcastFailed (rendered: {rendered})" + ); + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert_eq!(msg, rendered, "Display payload must survive verbatim"); + + let unconfirmed = PlatformWalletError::ShieldedBroadcastUnconfirmed { + identity_id: dpp::prelude::Identifier::from([7u8; 32]), + reason: "result proof fetch failed".to_string(), + }; + let rendered = unconfirmed.to_string(); + let result: PlatformWalletFFIResult = unconfirmed.into(); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorShieldedBroadcastUnconfirmed, + "ShieldedBroadcastUnconfirmed should map to ErrorShieldedBroadcastUnconfirmed (rendered: {rendered})" + ); + let msg = unsafe { std::ffi::CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned(); + assert_eq!(msg, rendered, "Display payload must survive verbatim"); + } + /// Other wallet-error variants without a dedicated FFI arm still /// fall through to `ErrorUnknown` while carrying the typed /// Display rendering as the message. Pin this so the catch-all diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index f39405d2dee..f74b6bd9de3 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -798,8 +798,10 @@ where .await { Ok(result) => result, - Err(dash_sdk::Error::StateTransitionBroadcastError(e)) => { - return Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())); + Err(wait_err) if wait_error_is_definitive_rejection(&wait_err) => { + return Err(PlatformWalletError::ShieldedBroadcastFailed( + wait_err.to_string(), + )); } Err(wait_err) => { warn!( @@ -930,6 +932,21 @@ where } } +/// Whether a wait-stage error is a DEFINITIVE platform rejection — the transition executed and was +/// rejected on its merits, so the identity does not exist and releasing the note reservations is +/// correct — as opposed to an AMBIGUOUS confirmation failure, where the broadcast was accepted and +/// the transition may have executed even though we couldn't fetch/verify its result proof (so the +/// reservation must be retained until a direct id fetch or the next nullifier sync resolves it). +/// +/// Only `StateTransitionBroadcastError` is definitive: it carries Platform's own report of the +/// transition's execution error. Everything else — `DriveProofError` / `Proof` / +/// `InvalidProvedResponse` / `TimeoutReached` / `DapiClientError` / `Config` / … — is ambiguous +/// (this is exactly the #3859 result-proof incident: the transition landed but the result proof +/// couldn't be confirmed). +fn wait_error_is_definitive_rejection(e: &dash_sdk::Error) -> bool { + matches!(e, dash_sdk::Error::StateTransitionBroadcastError(_)) +} + /// Number of times [`identity_create_from_shielded_pool`] re-fetches the new identity by its /// derived id after a post-broadcast result-confirmation failure, before declaring the broadcast /// unconfirmed. @@ -1306,3 +1323,49 @@ mod reserve_shield_fee_tests { assert!(matches!(err, PlatformWalletError::ShieldedBuildError(_))); } } + +#[cfg(test)] +mod wait_error_classification_tests { + use super::*; + + /// `StateTransitionBroadcastError` is the one wait-stage error Platform raises after the + /// transition *ran* and was rejected on its merits. It must be classified definitive so the + /// caller releases the note reservations (the identity does not exist). + #[test] + fn state_transition_broadcast_error_is_definitive() { + let e = dash_sdk::Error::StateTransitionBroadcastError( + dash_sdk::error::StateTransitionBroadcastError { + code: 1, + message: "rejected on merits".to_string(), + cause: None, + }, + ); + assert!( + wait_error_is_definitive_rejection(&e), + "StateTransitionBroadcastError must be treated as a definitive rejection" + ); + } + + /// Every other wait-stage error is ambiguous: the broadcast was accepted, so the transition may + /// have executed even though the result proof couldn't be fetched/verified. Classifying any of + /// these definitive would release reservations for notes that may already be spent on chain — + /// the orphaned-identity + double-spend hazard. Cover the result-proof shapes (#3859) plus a + /// timeout and a misconfiguration to pin the table. + #[test] + fn other_wait_errors_are_ambiguous() { + let ambiguous: Vec = vec![ + dash_sdk::Error::TimeoutReached( + std::time::Duration::from_secs(30), + "wait timed out".to_string(), + ), + dash_sdk::Error::InvalidProvedResponse("result proof did not verify".to_string()), + dash_sdk::Error::Config("misconfigured sdk".to_string()), + ]; + for e in &ambiguous { + assert!( + !wait_error_is_definitive_rejection(e), + "{e:?} must be treated as ambiguous (reservation retained)" + ); + } + } +} diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift index 4f89499cfdc..86ee5b1898a 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift @@ -100,15 +100,19 @@ final class IdentityRegistrationController: ObservableObject { /// Which stage a shielded `.failed` terminal state failed at, so /// `RegistrationProgressView` can attribute the red marker to the - /// right step instead of always blaming the Halo 2 proof step. - /// Only meaningful for `fundingKind == .shieldedPool` and only set - /// alongside a `.failed` phase. + /// right step. Only meaningful for `fundingKind == .shieldedPool`. + /// + /// Deliberately only ONE case: `.broadcastRejected` is the single + /// failure shape we can attribute with confidence (a typed + /// `PlatformWalletError.shieldedBroadcastFailed`). We cannot honestly + /// claim a "before broadcast" stage by elimination — Rust build/proof + /// errors arrive as a generic `.walletOperation`, indistinguishable at + /// this layer from an invalid handle, a marshalling failure, or any other + /// non-broadcast error. So `failureStage` is left `nil` for everything + /// else and the progress view falls back to its elapsed-time heuristic. + /// Extensible: add a case here only when a new typed error actually lets + /// us attribute the stage with certainty. enum FailureStage { - /// Failed before or during the broadcast itself — build / proof - /// error, or a relay/CheckTx broadcast rejection. The shielded - /// progress view keeps the existing elapsed-time heuristic - /// (note-selection vs Halo 2 proof) for the pre-broadcast slice. - case beforeBroadcast /// Platform definitively rejected the broadcast transition (a /// `PlatformWalletError.shieldedBroadcastFailed`). Attributed to /// the "Broadcasting transition" step. @@ -120,9 +124,13 @@ final class IdentityRegistrationController: ObservableObject { /// `.completed(id) | .failed(message) | .unconfirmed(id, message)`. @Published private(set) var phase: Phase = .idle - /// Stage attribution for a shielded `.failed` phase. `nil` whenever - /// the phase is not a shielded failure. Reset at the start of every - /// `submit` so a retry doesn't inherit the previous attempt's stage. + /// Stage attribution for a shielded `.failed` phase. `nil` means + /// "unattributed" — either the phase isn't a shielded failure, or the + /// failure wasn't a confidently-attributable broadcast rejection. On a + /// `nil` shielded failure the progress view falls back to its + /// elapsed-time heuristic (note-selection vs Halo 2 proof). Reset at the + /// start of every `submit` so a retry doesn't inherit the previous + /// attempt's stage. @Published private(set) var failureStage: FailureStage? /// Slot this controller is bound to. Stored so the coordinator @@ -195,8 +203,11 @@ final class IdentityRegistrationController: ObservableObject { /// `body` performs the actual FFI call. It runs detached on a /// background priority and reports the identity id on success /// or rethrows on failure. The controller flips `phase` to - /// `.completed` / `.unconfirmed` / `.failed` accordingly, and on a - /// shielded `.failed` records `failureStage` for step attribution. + /// `.completed` / `.unconfirmed` / `.failed` accordingly. On a shielded + /// `.failed` it records `failureStage = .broadcastRejected` ONLY for a + /// `PlatformWalletError.shieldedBroadcastFailed`; every other error leaves + /// `failureStage` nil (unattributed — the progress view uses its + /// elapsed-time heuristic) rather than guessing a stage by elimination. func submit(body: @escaping () async throws -> Data) { switch phase { case .idle, .preparingKeys, .failed: @@ -229,16 +240,17 @@ final class IdentityRegistrationController: ObservableObject { ) } } catch { - // Attribute the failure stage so the shielded progress view - // can point the red marker at the right step. A - // `shieldedBroadcastFailed` is a definitive platform/relay - // rejection of the broadcast; everything else (build / Halo 2 - // proof errors) failed before the broadcast. - let stage: FailureStage + // Only a `shieldedBroadcastFailed` is a confidently-attributable + // broadcast rejection → mark the broadcast step. Everything else + // (Rust build / Halo 2 proof errors, which surface as a generic + // `.walletOperation`, plus invalid-handle / marshalling failures) + // is indistinguishable at this layer, so leave `failureStage` nil + // and let the progress view's elapsed-time heuristic place it. + let stage: FailureStage? if case PlatformWalletError.shieldedBroadcastFailed = error { stage = .broadcastRejected } else { - stage = .beforeBroadcast + stage = nil } await MainActor.run { self?.failureStage = stage diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift index dd1f4619210..5ffbfe4748a 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift @@ -1463,15 +1463,22 @@ struct CreateIdentityView: View { // the inline failure state. return case .unconfirmed: - // Broadcast landed but its result couldn't be - // confirmed; the identity is probably already live on - // chain. Mark the slot used (same as `.completed`) so - // the next registration can't reuse these keys and - // burn funds against the registered-key-hash stateful - // check. We do NOT persist a `PersistentIdentity` row - // here — the proof-verified identity wasn't returned; - // the next sync writes the row once the identity is - // confirmed on chain. + // Broadcast landed but its result couldn't be confirmed; + // the identity is probably already live on chain. We do NOT + // persist a `PersistentIdentity` row here — the + // proof-verified identity wasn't returned; the next sync + // writes the row once the identity is confirmed on chain. + // + // `markIdentitySlotUsed` is called for consistency with + // `.completed`, but it does NOT actually protect the slot: + // it only flips the deprecated `PersistentCoreAddress.isUsed` + // flag, which the picker's `usedIdentityIndices` ignores + // (that reads persisted `PersistentIdentity` rows only). The + // real guards keeping this index un-selectable until the + // identity row appears are the live `.unconfirmed` + // controller (held in the coordinator) and the + // `.unconfirmed` dismissibility gate in + // `PendingRegistrationsList`. markIdentitySlotUsed( walletId: walletId, identityIndex: identityIndex diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift index 3ccb6880e15..ab1e7151c94 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift @@ -1,4 +1,5 @@ import SwiftUI +import SwiftData import SwiftDashSDK /// Section wrapper that observes a `RegistrationCoordinator` directly @@ -51,6 +52,28 @@ struct PendingRegistrationRow: View { @ObservedObject var controller: IdentityRegistrationController @EnvironmentObject var walletManager: PlatformWalletManager + /// Persisted identity rows for this slot, queried live so the + /// `.unconfirmed` dismiss gate becomes enabled the moment the + /// identity-sync writes the `PersistentIdentity` row. Filtered by + /// `(wallet.walletId, identityIndex)` — the same `(walletId, + /// identityIndex)` slot key `RegistrationProgressSection` uses to + /// query its `PersistentAssetLock` row. `controller.walletId` / + /// `controller.identityIndex` are immutable `let`s, so the predicate + /// captured in `init` stays correct for the row's lifetime. + @Query private var slotIdentities: [PersistentIdentity] + + init(controller: IdentityRegistrationController) { + self.controller = controller + let walletId = controller.walletId + let identityIndex = controller.identityIndex + _slotIdentities = Query( + filter: #Predicate { identity in + identity.wallet?.walletId == walletId + && identity.identityIndex == identityIndex + } + ) + } + var body: some View { NavigationLink(destination: RegistrationProgressView(controller: controller)) { VStack(alignment: .leading, spacing: 4) { @@ -72,10 +95,14 @@ struct PendingRegistrationRow: View { .padding(.vertical, 2) } .swipeActions(edge: .trailing, allowsFullSwipe: false) { - // Both terminal-but-retained states are user-dismissable. - // `.unconfirmed` stays until the user clears it (typically once - // the identity row appears via sync); dismissing it only drops - // the Pending row, it doesn't undo the on-chain registration. + // `.failed` is always dismissable. `.unconfirmed` only becomes + // dismissable once the matching `PersistentIdentity` row appears + // via sync (see `isDismissable`): while unconfirmed-and-unpersisted + // the live controller is the ONLY guard keeping the slot + // un-selectable, so dismissing it would let the same index be + // re-selected and burn funds against the registered-key-hash check. + // Dismissing only drops the Pending row; it never undoes the + // on-chain registration. if isDismissable { Button { walletManager.registrationCoordinator.dismiss( @@ -92,8 +119,22 @@ struct PendingRegistrationRow: View { private var isDismissable: Bool { switch controller.phase { - case .failed, .unconfirmed: return true - default: return false + case .failed: + // The user is expected to read the error and retry; always + // dismissable. + return true + case .unconfirmed: + // The slot is held to block a re-submission that would burn funds + // (the identity is probably live on chain). The picker's + // `usedIdentityIndices` consults ONLY persisted `PersistentIdentity` + // rows, not the controller, so the live controller is the ONLY + // thing keeping this index un-selectable until that row lands. + // Allow dismiss only once the identity-sync has written the + // `PersistentIdentity` row — after that the slot is protected by + // the persisted row and dropping the controller is safe. + return !slotIdentities.isEmpty + default: + return false } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift index 83c39414dd3..ca36b281edb 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/RegistrationProgressView.swift @@ -253,13 +253,15 @@ struct RegistrationProgressSection: View { /// On `.completed` return 6 (one past the last step) so all rows /// render `.done`. On `.unconfirmed` return 4 ("Waiting for platform /// confirmation") so that step carries the warning. On `.failed` - /// attribute the step: a `broadcastRejected` failure marks step 3 - /// ("Broadcasting transition"); anything else keeps the - /// note-selection vs Halo 2 elapsed-time heuristic, measured *at - /// the failure instant* (anchored on `controller.terminalAt`) — - /// failed rows are retained until dismissed, so measuring against - /// live `now` would let the failed icon drift from step 1 to - /// step 2 once the note-selection window lapses on the wall clock. + /// attribute the step: a `.broadcastRejected` failure marks step 3 + /// ("Broadcasting transition"); an UNATTRIBUTED failure + /// (`failureStage == nil` — build / Halo 2 proof errors, or any other + /// error we can't confidently place) keeps the note-selection vs Halo 2 + /// elapsed-time heuristic, measured *at the failure instant* (anchored + /// on `controller.terminalAt`) — failed rows are retained until + /// dismissed, so measuring against live `now` would let the failed icon + /// drift from step 1 to step 2 once the note-selection window lapses on + /// the wall clock. private func shieldedCurrentStep(now: Date) -> Int { switch controller.phase { case .idle, .preparingKeys: @@ -273,12 +275,14 @@ struct RegistrationProgressSection: View { case .inFlight: return shieldedStep(elapsedTo: now) case .failed: - // A definitive broadcast rejection is attributed to the - // broadcast step (3). Build / Halo 2 proof errors fail before - // the broadcast, so keep the elapsed-time heuristic - // (note-selection vs proof) for them — frozen at the failure - // instant; fall back to `now` only if the terminal timestamp - // is missing (pre-submit failure shapes never set it). + // A definitive broadcast rejection (`failureStage == + // .broadcastRejected`) is attributed to the broadcast step (3). + // For an unattributed failure (`failureStage == nil` — build / + // Halo 2 proof errors, or any other error we can't confidently + // place), keep the elapsed-time heuristic (note-selection vs + // proof) — frozen at the failure instant; fall back to `now` only + // if the terminal timestamp is missing (pre-submit failure shapes + // never set it). if controller.failureStage == .broadcastRejected { return 3 } From d9283d0ce3bad03db8a8fa23548f3176ca3ad26c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:49:17 +0200 Subject: [PATCH 3/3] fix(swift-sdk): hold network-switch gate for unconfirmed slots and pin reservation retention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the second thepastaclaw review round on #3862: - example app (blocking): hasInFlightRegistrations now reads controller.phase.isActive instead of re-listing cases, so an .unconfirmed slot keeps the OptionsView network toggle disabled. Switching networks tears down the PlatformWalletManager and with it the coordinator — dropping the only in-memory guard keeping the slot un-selectable before the PersistentIdentity row lands. New unit test pins the gate-equals-isActive equivalence through the public startRegistration path. - platform-wallet: the outer-match reservation decision is now the named error_releases_note_reservation helper (false only for ShieldedBroadcastUnconfirmed), unit-tested so a refactor can't silently fold the unconfirmed arm into the generic cancellation branch. Co-Authored-By: Claude Fable 5 --- .../src/wallet/shielded/operations.rs | 68 +++++++++++++++--- .../Services/RegistrationCoordinator.swift | 38 ++++++---- .../CreateIdentityResumableTests.swift | 71 +++++++++++++++++++ 3 files changed, 155 insertions(+), 22 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index f74b6bd9de3..b1f7c08b351 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -916,22 +916,32 @@ where ); Ok((identity_id, identity)) } - // The broadcast was accepted but its result couldn't be confirmed and the identity wasn't - // found by a direct fetch. Do NOT `cancel_pending` here: `pending_nullifiers` is in-memory - // only (see `SubwalletState`, "never persisted; the next sync after a crash reconciles"), - // and `mark_spent` during nullifier sync clears matching reservations. So if the transition - // actually executed, the next sync promotes these notes to spent; if it truly never landed, - // an app restart drops the in-memory reservation and frees them. Releasing them now would - // invite double-spend attempts against notes that may already be consumed on chain — the - // very hazard this variant exists to prevent. - Err(e @ PlatformWalletError::ShieldedBroadcastUnconfirmed { .. }) => Err(e), Err(e) => { - cancel_pending(store, id, &selected_notes).await; + if error_releases_note_reservation(&e) { + cancel_pending(store, id, &selected_notes).await; + } Err(e) } } } +/// Whether a failed identity-create should release the notes reserved for it. +/// +/// `false` ONLY for [`PlatformWalletError::ShieldedBroadcastUnconfirmed`]: the broadcast was +/// accepted and the transition may have executed, so the reservation must be retained. Releasing it +/// now would invite double-spend attempts against notes that may already be consumed on chain — the +/// very hazard that variant exists to prevent. `pending_nullifiers` is in-memory only (see +/// `SubwalletState`, "never persisted; the next sync after a crash reconciles") and `mark_spent` +/// during nullifier sync clears matching reservations, so if the transition actually executed the +/// next sync promotes these notes to spent; if it truly never landed, an app restart drops the +/// in-memory reservation and frees them. +/// +/// Everything else is a definitive pre-execution / build / rejection failure: the spend never +/// happened, so the reservation must be released. +fn error_releases_note_reservation(e: &PlatformWalletError) -> bool { + !matches!(e, PlatformWalletError::ShieldedBroadcastUnconfirmed { .. }) +} + /// Whether a wait-stage error is a DEFINITIVE platform rejection — the transition executed and was /// rejected on its merits, so the identity does not exist and releasing the note reservations is /// correct — as opposed to an AMBIGUOUS confirmation failure, where the broadcast was accepted and @@ -1369,3 +1379,41 @@ mod wait_error_classification_tests { } } } + +#[cfg(test)] +mod note_reservation_release_tests { + use super::*; + + /// `ShieldedBroadcastUnconfirmed` is the one failure that must NOT release the reservation: the + /// broadcast was accepted and the transition may have executed, so freeing the notes invites a + /// double-spend against notes that may already be consumed on chain. The next nullifier sync + /// reconciles them. + #[test] + fn unconfirmed_broadcast_retains_reservation() { + let e = PlatformWalletError::ShieldedBroadcastUnconfirmed { + identity_id: Identifier::from([7u8; 32]), + reason: "result proof unavailable".to_string(), + }; + assert!( + !error_releases_note_reservation(&e), + "ShieldedBroadcastUnconfirmed must retain the note reservation" + ); + } + + /// Every other failure is a definitive pre-execution / build / rejection failure — the spend + /// never happened, so the reservation must be released. + #[test] + fn definitive_failures_release_reservation() { + let releasing: Vec = vec![ + PlatformWalletError::ShieldedBroadcastFailed("rejected on merits".to_string()), + PlatformWalletError::ShieldedBuildError("note selection failed".to_string()), + PlatformWalletError::ShieldedStoreError("store write failed".to_string()), + ]; + for e in &releasing { + assert!( + error_releases_note_reservation(e), + "{e:?} must release the note reservation" + ); + } + } +} diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift index e3fcba18dc9..7051085c1ba 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/RegistrationCoordinator.swift @@ -35,20 +35,34 @@ final class RegistrationCoordinator: ObservableObject { /// observe map mutations via `objectWillChange`. @Published private(set) var controllers: [SlotKey: IdentityRegistrationController] = [:] - /// True when at least one slot is currently in flight (phase - /// `.preparingKeys` or `.inFlight`). Used by the network - /// toggle's `.disabled(_:)` modifier — switching testnet ↔ - /// mainnet mid-flight tears down the FFI manager and would - /// abort the in-flight call mid-stream. The UI guards against - /// that race by reading this flag. + /// True when at least one slot is still holding itself against a + /// fresh registration — exactly `controller.phase.isActive` + /// (`.preparingKeys`, `.inFlight`, or `.unconfirmed`). Used by the + /// network toggle's `.disabled(_:)` modifier. + /// + /// Two reasons a slot must hold this gate: + /// - `.preparingKeys` / `.inFlight`: switching testnet ↔ mainnet + /// mid-flight tears down the FFI manager and would abort the + /// in-flight call mid-stream. + /// - `.unconfirmed`: the identity is probably live on chain, but the + /// picker's `usedIdentityIndices` consults ONLY persisted + /// `PersistentIdentity` rows — so the live controller is the ONLY + /// thing keeping the slot un-selectable until that row lands via + /// sync. Switching networks tears down the `PlatformWalletManager` + /// and with it this coordinator, dropping the controller (and the + /// Rust-side note reservation); the same HD slot would become + /// selectable and a re-submission would be rejected by the + /// registered-key-hash stateful check and burn the funded spend. + /// + /// Reading `isActive` directly (rather than re-listing the cases) + /// keeps this gate from drifting from the phase model, mirroring + /// `PendingRegistrationsList.isDismissable`. UX trade-off, by design + /// (same as the dismissal gate): an `.unconfirmed` row blocks network + /// switching until it becomes dismissable (the identity row arrives + /// via sync) or the app restarts. var hasInFlightRegistrations: Bool { controllers.contains { _, controller in - switch controller.phase { - case .preparingKeys, .inFlight: - return true - default: - return false - } + controller.phase.isActive } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift index c17cad58eb5..d24354fb320 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/CreateIdentityResumableTests.swift @@ -303,4 +303,75 @@ final class CreateIdentityResumableTests: XCTestCase { ".unconfirmed: identity is probably live on chain — keep the slot held so a re-submission can't burn funds against the registered-key-hash check" ) } + + // MARK: - network-switch gate + + /// The network picker's `.disabled(_:)` gate reads + /// `RegistrationCoordinator.hasInFlightRegistrations`. That predicate + /// must hold for an `.unconfirmed` slot (the same reason the dismissal + /// gate does): switching networks tears down the + /// `PlatformWalletManager` and with it the coordinator + the Rust-side + /// note reservation, so releasing the gate for an `.unconfirmed` + /// controller lets the same HD slot be re-selected and burn funds + /// against the registered-key-hash check. + /// + /// The gate is implemented as `controller.phase.isActive` so it + /// cannot list a different set of phases than the slot-occupancy + /// model. Reaching `.unconfirmed` directly requires throwing the + /// SDK's `ShieldedIdentityCreateUnconfirmedError`, whose initializer + /// is `internal` to `SwiftDashSDK` and not constructible here — so + /// the `.unconfirmed → gate held` leg is pinned transitively: the + /// exhaustive `testControllerPhaseIsActivePredicate` already asserts + /// `.unconfirmed.isActive == true`, and this test pins that the gate + /// is exactly `isActive` over the map (true iff some controller is + /// active, false when all are terminal-non-active). + @MainActor + func testNetworkSwitchGateMatchesPhaseIsActive() async throws { + let coordinator = RegistrationCoordinator() + let walletId = Data(repeating: 0xAB, count: 32) + + XCTAssertFalse( + coordinator.hasInFlightRegistrations, + "empty coordinator holds nothing in flight" + ) + + // A controller that fails terminally is `.failed` (isActive == + // false): the gate must release once it's the only entry. + let failing = coordinator.startRegistration( + walletId: walletId, + identityIndex: 1, + fundingKind: .shieldedPool + ) { + throw PlatformWalletError.shieldedBroadcastFailed("rejected on merits") + } + for _ in 0..<200 { + if case .failed = failing.phase { break } + try await Task.sleep(nanoseconds: 10_000_000) + } + guard case .failed = failing.phase else { + return XCTFail("controller did not reach .failed in time") + } + XCTAssertFalse(failing.phase.isActive) + XCTAssertFalse( + coordinator.hasInFlightRegistrations, + "a lone .failed controller must not hold the network-switch gate (it's not isActive)" + ) + + // A controller stuck `.inFlight` (isActive == true) — the gate + // must hold. Use a never-returning body so the phase stays + // `.inFlight` for the duration of the assertion. + let inFlight = coordinator.startRegistration( + walletId: walletId, + identityIndex: 2, + fundingKind: .shieldedPool + ) { + try await Task.sleep(nanoseconds: 60_000_000_000) + return Data() + } + XCTAssertTrue(inFlight.phase.isActive) + XCTAssertTrue( + coordinator.hasInFlightRegistrations, + "an active (.inFlight) controller must hold the network-switch gate; the gate is exactly phase.isActive, which also covers .unconfirmed" + ) + } }