From 39c6053dc92863ac060b299e0ec063116e77f9e7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 11 Jun 2026 23:56:32 +0200 Subject: [PATCH 1/8] 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 | 79 ++++++- .../Services/RegistrationCoordinator.swift | 20 +- .../Views/CreateIdentityView.swift | 59 +++++- .../Views/PendingRegistrationsList.swift | 16 +- .../Views/RegistrationProgressView.swift | 196 ++++++++++++++---- .../CreateIdentityResumableTests.swift | 9 + 13 files changed, 655 insertions(+), 68 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 491cda0b722..513efed63bc 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)`. @@ -142,6 +183,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()`, @@ -150,15 +194,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() terminalAt = nil task = Task { [weak self] in @@ -168,8 +216,31 @@ final class IdentityRegistrationController: ObservableObject { self?.terminalAt = Date() 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?.terminalAt = Date() + 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?.terminalAt = Date() 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 7b2a536b3f9..e6f51019f36 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,39 +235,54 @@ 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 *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. + /// 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, anchored on + /// `controller.terminalAt` (the failure instant) — 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: 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: return shieldedStep(elapsedTo: now) case .failed: - // Freeze 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 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, falling back to `now` only if the terminal + // timestamp is missing (pre-submit failure shapes never set + // it). + if controller.failureStage == .broadcastRejected { + return 3 + } return shieldedStep(elapsedTo: controller.terminalAt ?? now) } } @@ -303,6 +343,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 @@ -314,7 +362,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 "" } } @@ -333,12 +382,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 @@ -414,6 +479,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) } } @@ -424,10 +495,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." } @@ -436,7 +514,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 "" } } @@ -521,6 +600,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 c8c68c93d78ce330f5a43b9fbc079ccfde3344b5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 00:16:57 +0200 Subject: [PATCH 2/8] fix(platform-wallet): keep note reservations on ambiguous shielded spend confirmation failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unshield, shielded transfer, and shielded withdrawal used the one-shot broadcast_and_wait and ran cancel_pending on ANY error, including post-broadcast wait failures (result-proof fetch/verify errors, timeouts, transport errors) where the relay had already accepted the transition and the spend may well have executed. Releasing the in-memory note reservations there invites re-selecting notes whose nullifiers may already be consumed on chain. Apply the same stage-split + error classification as the Type-20 identity-create flow: broadcast() and wait_for_response() are staged separately via a shared broadcast_shielded_spend helper. Broadcast-time rejections and StateTransitionBroadcastError (Platform ran the transition and rejected it on its merits) keep today's ShieldedBroadcastFailed + cancel_pending; any other wait failure maps to the new ShieldedSpendUnconfirmed variant, whose outer-match arm leaves the reservation in place — pending_nullifiers is in-memory only, so the next nullifier sync promotes the notes to spent if the spend landed, and an app restart frees them if it never did. Shield (Type 15) and ShieldFromAssetLock (Type 18) deliberately keep the one-shot helper: they take no note reservation, so an ambiguous wait failure has no local state to strand (documented at the shield broadcast site). Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet/src/error.rs | 21 ++++ .../src/wallet/shielded/operations.rs | 110 ++++++++++++++---- 2 files changed, 110 insertions(+), 21 deletions(-) diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 57c0519e2d5..598b1b42838 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -202,6 +202,27 @@ pub enum PlatformWalletError { reason: String, }, + /// A shielded spend transition (`operation` is `"unshield"`, `"transfer"` or `"withdraw"`) 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 or timeout, not a + /// platform rejection). The spend may already be executed on chain, so the spent notes' + /// reservations are intentionally left in place rather than released — releasing them would + /// invite re-selecting notes whose nullifiers may already be consumed. The next nullifier sync + /// (or an app restart, since reservations are in-memory only) reconciles them. `reason` carries + /// the underlying SDK error for diagnostics. + /// + /// The identity-create sibling is [`Self::ShieldedBroadcastUnconfirmed`], which additionally + /// carries the derived identity id so the caller can hold the registration slot. + #[error( + "Shielded {operation} broadcast succeeded but its execution result could not be \ + confirmed; the spend may already be executed on chain — do not re-submit \ + (the next sync reconciles the spent notes): {reason}" + )] + ShieldedSpendUnconfirmed { + operation: &'static str, + 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 f39405d2dee..c0ca91f15ea 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -52,6 +52,7 @@ use dpp::shielded::builder::{ use dpp::shielded::compute_minimum_shielded_fee; use dpp::state_transition::proof_result::StateTransitionProofResult; use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; +use dpp::state_transition::StateTransition; use dpp::withdrawal::Pooling; use grovedb_commitment_tree::{Anchor, PaymentAddress}; use tokio::sync::RwLock; @@ -301,11 +302,15 @@ pub async fn shield, P: OrchardProver>( trace!("Shield credits: state transition built, broadcasting..."); let network = sdk.network; // Wait for proven execution (not just relay-ACK) so the host only - // sees success once Platform has actually included the transition — - // matching the spend-side flows (unshield/transfer/withdraw). A + // sees success once Platform has actually included the transition. A // DAPI-level ACK alone could otherwise mask a later Platform // rejection. The proven result is discarded; we only need the - // confirmation. + // confirmation. Unlike the note-spending flows (unshield / transfer / + // withdraw — see `broadcast_shielded_spend`), shield deliberately + // keeps the one-shot helper: it takes no note reservation, so an + // ambiguous post-broadcast wait failure has no local state to strand. + // Its inputs are transparent address claims guarded by on-chain + // nonces, and a host-level retry re-fetches those nonces. state_transition .broadcast_and_wait::(sdk, None) .await @@ -383,8 +388,10 @@ pub async fn unshield( "Unshield" ); - // From here on every error path must release the reservation - // taken by `reserve_unspent_notes`. + // From here on every error path must release the reservation taken + // by `reserve_unspent_notes` — except the ambiguous + // `ShieldedSpendUnconfirmed` one, which intentionally leaves it in + // place (see the outer match below). let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; @@ -411,11 +418,7 @@ pub async fn unshield( ); trace!("Unshield: state transition built, broadcasting..."); - state_transition - .broadcast_and_wait::(sdk, None) - .await - .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; - Ok::<(), PlatformWalletError>(()) + broadcast_shielded_spend(sdk, &state_transition, "unshield").await } .await; @@ -448,6 +451,15 @@ pub async fn unshield( info!(account, credits = amount, "Unshield broadcast succeeded"); Ok(()) } + // The broadcast was accepted but its execution result couldn't be + // confirmed: the spend may well have executed, so do NOT release + // the reservation. `pending_nullifiers` is in-memory only — if the + // spend landed, the next nullifier sync's `mark_spent` promotes the + // notes to spent (clearing the reservation); if it never landed, an + // app restart drops the reservation and frees the notes. Releasing + // them now would invite re-selecting notes whose nullifiers may + // already be consumed on chain. + Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { cancel_pending(store, id, &selected_notes).await; Err(e) @@ -518,11 +530,7 @@ pub async fn transfer( ); trace!("Shielded transfer: state transition built, broadcasting..."); - state_transition - .broadcast_and_wait::(sdk, None) - .await - .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; - Ok::<(), PlatformWalletError>(()) + broadcast_shielded_spend(sdk, &state_transition, "transfer").await } .await; @@ -545,6 +553,9 @@ pub async fn transfer( ); Ok(()) } + // Ambiguous post-broadcast confirmation failure: leave the + // reservation in place (see `unshield`'s outer match). + Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { cancel_pending(store, id, &selected_notes).await; Err(e) @@ -623,11 +634,7 @@ pub async fn withdraw( ); trace!("Shielded withdrawal: state transition built, broadcasting..."); - state_transition - .broadcast_and_wait::(sdk, None) - .await - .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; - Ok::<(), PlatformWalletError>(()) + broadcast_shielded_spend(sdk, &state_transition, "withdraw").await } .await; @@ -650,6 +657,9 @@ pub async fn withdraw( ); Ok(()) } + // Ambiguous post-broadcast confirmation failure: leave the + // reservation in place (see `unshield`'s outer match). + Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => Err(e), Err(e) => { cancel_pending(store, id, &selected_notes).await; Err(e) @@ -734,7 +744,9 @@ where .map(|(key, _)| (key.id(), key.clone())) .collect(); - // From here on every error path must release the reservation taken above. + // From here on every error path must release the reservation taken above — except the + // ambiguous `ShieldedBroadcastUnconfirmed` one, which intentionally leaves it in place + // (see the outer match below). let result = async { let (spends, anchor) = extract_spends_and_anchor(store, &selected_notes).await?; @@ -1209,6 +1221,62 @@ async fn cancel_pending( } } +/// Broadcast a built shielded spend transition (unshield / transfer / +/// withdraw) and wait for proven execution, staging the two SDK calls +/// separately so the caller's reservation rollback only runs when the +/// spend DEFINITIVELY did not happen: +/// +/// - a broadcast-time rejection (relay/CheckTx refused the tx), or a +/// `StateTransitionBroadcastError` from the result wait (Platform ran +/// the transition and rejected it on its merits), means the spend +/// never executed → [`PlatformWalletError::ShieldedBroadcastFailed`], +/// and the caller releases the note reservations via +/// [`cancel_pending`]; +/// - any other wait failure (result-proof fetch/verify error, timeout, +/// transport error, …) is AMBIGUOUS — the relay accepted the tx and +/// it may well have executed → +/// [`PlatformWalletError::ShieldedSpendUnconfirmed`], and the caller +/// must leave the reservations in place (each spend flow's outer +/// match has a dedicated arm for this). +/// +/// Mirrors the staging in [`identity_create_from_shielded_pool`], minus +/// its fetch-by-derived-id fallback: a spend leaves no artifact as +/// cheaply queryable as an identity row, so ambiguity is surfaced +/// directly and reconciled by the next nullifier sync. The proven +/// result is discarded; only the confirmation matters. +async fn broadcast_shielded_spend( + sdk: &Arc, + state_transition: &StateTransition, + operation: &'static str, +) -> Result<(), PlatformWalletError> { + state_transition + .broadcast(sdk, None) + .await + .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; + + match state_transition + .wait_for_response::(sdk, None) + .await + { + Ok(_) => Ok(()), + Err(dash_sdk::Error::StateTransitionBroadcastError(e)) => { + Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())) + } + Err(wait_err) => { + warn!( + operation, + error = %wait_err, + "Shielded spend broadcast accepted but result confirmation failed; \ + leaving the note reservations in place" + ); + Err(PlatformWalletError::ShieldedSpendUnconfirmed { + operation, + reason: wait_err.to_string(), + }) + } + } +} + /// Helper to clone selection results out from under the store lock. trait SelectionResultOwned { fn into_owned(self) -> (Vec, u64, u64); From 327240c952dbaa6e4a3d8eee15ba9c27f0fbc3b0 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:04:22 +0200 Subject: [PATCH 3/8] fix(platform-wallet): treat cause-less broadcast errors as unconfirmed and type spend results over FFI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups from #3863: DAPI encodes its own wait-side failures (DapiError::Timeout, internal errors) as StateTransitionBroadcastError with empty consensus data (build_wait_for_state_transition_error_response), which the SDK decodes as cause: None. The previous classification treated every StateTransitionBroadcastError as a definitive Platform rejection, so a DAPI wait timeout still released note reservations — exactly the ambiguous case this PR protects. Gate the definitive arm on cause.is_some() in both broadcast_shielded_spend and the identity-create flow (where cause-less errors now reach the fetch-by-derived-id fallback instead of failing hard). Misreading a rejection as ambiguous only delays note release until the next sync or restart, so the gate errs in the safe direction. Adds unit tests for the classification. Also surface ShieldedSpendUnconfirmed as a dedicated FFI result code (ErrorShieldedSpendUnconfirmed = 18) in the three spend FFIs, mirrored in the Swift PlatformWalletResultCode/PlatformWalletError enums, so hosts can tell "may have executed, do NOT retry" from a retryable failure instead of both collapsing into ErrorWalletOperation — a host retry on the unconfirmed path could select different unreserved notes and double-send. ShieldedBroadcastFailed now maps to the existing code 16 for spends, matching the identity-create sibling. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet-ffi/src/error.rs | 21 ++- .../src/shielded_send.rs | 53 ++++--- .../src/wallet/shielded/operations.rs | 131 +++++++++++++++--- .../PlatformWallet/PlatformWalletResult.swift | 17 ++- 4 files changed, 178 insertions(+), 44 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 9a243372713..8c6412cb032 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -98,11 +98,12 @@ pub enum PlatformWalletFFIResultCode { /// 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). + /// transition (identity-create, unshield, transfer, or withdrawal) was + /// DEFINITIVELY not executed — either the relay/CheckTx rejected the + /// broadcast, or Platform reported the transition's own execution error. + /// Any note reservations were released and the caller is free to retry. + /// For identity-create, the new identity does NOT exist and + /// `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 @@ -113,6 +114,16 @@ pub enum PlatformWalletFFIResultCode { /// `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, + /// Maps `PlatformWalletError::ShieldedSpendUnconfirmed` (unshield / + /// shielded transfer / shielded withdrawal). The spend transition was + /// ACCEPTED by the relay but its execution result could not be confirmed + /// (DAPI wait timeout, result-proof fetch/verify failure, …). The spend + /// may have executed on chain, so the wallet intentionally KEEPS the + /// notes reserved: the next nullifier sync promotes them to spent if the + /// spend landed, and an app restart frees them if it never did. The host + /// must NOT auto-retry — a retry would select different unreserved notes + /// and could double-send if the original spend landed. + ErrorShieldedSpendUnconfirmed = 18, 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 90d2252f265..95269402d62 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -315,13 +315,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer( .shielded_transfer_to(&coordinator, account, &recipient, amount, memo, &prover) .await }); - if let Err(e) = result { - return PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorWalletOperation, - format!("shielded transfer failed: {e}"), - ); - } - PlatformWalletFFIResult::ok() + map_spend_result(result, "shielded transfer") } /// Unshield: spend shielded notes and send `amount` credits to a @@ -373,13 +367,7 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_unshield( .shielded_unshield_to(&coordinator, account, &to_addr_str, amount, &prover) .await }); - if let Err(e) = result { - return PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorWalletOperation, - format!("shielded unshield failed: {e}"), - ); - } - PlatformWalletFFIResult::ok() + map_spend_result(result, "shielded unshield") } /// Withdraw: spend shielded notes and send `amount` credits to a @@ -435,13 +423,40 @@ pub unsafe extern "C" fn platform_wallet_manager_shielded_withdraw( ) .await }); - if let Err(e) = result { - return PlatformWalletFFIResult::err( + map_spend_result(result, "shielded withdraw") +} + +/// Map a shielded spend outcome (unshield / transfer / withdraw) to a typed +/// FFI result, mirroring the identity-create sibling's code split so hosts +/// can tell "definitively failed, safe to retry" from "may have executed, +/// do NOT retry". +fn map_spend_result( + result: Result<(), PlatformWalletError>, + operation: &str, +) -> PlatformWalletFFIResult { + match result { + Ok(()) => PlatformWalletFFIResult::ok(), + // Ambiguous: the broadcast was accepted but its execution result + // couldn't be confirmed. The notes stay reserved wallet-side and the + // next nullifier sync (or an app restart) reconciles them; the typed + // Display already carries the operation name and guidance. + Err(e @ PlatformWalletError::ShieldedSpendUnconfirmed { .. }) => { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed, + e.to_string(), + ) + } + // Definitive failure: the transition was not executed and the notes + // were released; the host may retry. + Err(e @ PlatformWalletError::ShieldedBroadcastFailed(_)) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorShieldedBroadcastFailed, + format!("{operation} failed: {e}"), + ), + Err(e) => PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, - format!("shielded withdraw failed: {e}"), - ); + format!("{operation} failed: {e}"), + ), } - PlatformWalletFFIResult::ok() } /// IdentityCreateFromShieldedPool (Type 20): spend `account`'s shielded notes to fund a brand-new diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index c0ca91f15ea..0fe23d03cd2 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -797,20 +797,24 @@ where .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`). + // - `StateTransitionBroadcastError` WITH a consensus `cause` = Platform DEFINITIVELY + // reported the transition's own execution error (it ran and was rejected on its + // merits; the serialized consensus error is the verdict). 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. + // the #3859 result-proof incident). That includes cause-less + // `StateTransitionBroadcastError`s — DAPI encodes its own wait-side failures + // (timeouts, internal errors) that way, with empty consensus data. 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)) => { + Err(dash_sdk::Error::StateTransitionBroadcastError(e)) if e.cause.is_some() => { return Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())); } Err(wait_err) => { @@ -1227,11 +1231,11 @@ async fn cancel_pending( /// spend DEFINITIVELY did not happen: /// /// - a broadcast-time rejection (relay/CheckTx refused the tx), or a -/// `StateTransitionBroadcastError` from the result wait (Platform ran -/// the transition and rejected it on its merits), means the spend -/// never executed → [`PlatformWalletError::ShieldedBroadcastFailed`], -/// and the caller releases the note reservations via -/// [`cancel_pending`]; +/// `StateTransitionBroadcastError` carrying a consensus `cause` +/// (Platform ran the transition and rejected it on its merits), means +/// the spend never executed → +/// [`PlatformWalletError::ShieldedBroadcastFailed`], and the caller +/// releases the note reservations via [`cancel_pending`]; /// - any other wait failure (result-proof fetch/verify error, timeout, /// transport error, …) is AMBIGUOUS — the relay accepted the tx and /// it may well have executed → @@ -1254,25 +1258,47 @@ async fn broadcast_shielded_spend( .await .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; - match state_transition + state_transition .wait_for_response::(sdk, None) .await - { - Ok(_) => Ok(()), - Err(dash_sdk::Error::StateTransitionBroadcastError(e)) => { - Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())) + .map(|_| ()) + .map_err(|wait_err| classify_spend_wait_failure(operation, &wait_err)) +} + +/// Classify a `wait_for_response` failure for an already-broadcast +/// shielded spend (see [`broadcast_shielded_spend`]). +/// +/// Only a `StateTransitionBroadcastError` with a populated consensus +/// `cause` proves Platform executed the transition and rejected it on +/// its merits — the serialized consensus error is the verdict. DAPI +/// encodes its own wait-side failures (timeouts, internal errors; see +/// `build_wait_for_state_transition_error_response` in rs-dapi) as +/// `StateTransitionBroadcastError`s with EMPTY consensus data, which +/// the SDK surfaces as `cause: None` — those are ambiguous, not +/// rejections, and must keep the note reservations. Erring this way is +/// the safe direction: misreading a rejection as ambiguous only delays +/// note release until the next sync or restart, while misreading a +/// timeout as a rejection re-frees notes whose nullifiers may already +/// be consumed on chain. +fn classify_spend_wait_failure( + operation: &'static str, + wait_err: &dash_sdk::Error, +) -> PlatformWalletError { + match wait_err { + dash_sdk::Error::StateTransitionBroadcastError(e) if e.cause.is_some() => { + PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) } - Err(wait_err) => { + _ => { warn!( operation, error = %wait_err, "Shielded spend broadcast accepted but result confirmation failed; \ leaving the note reservations in place" ); - Err(PlatformWalletError::ShieldedSpendUnconfirmed { + PlatformWalletError::ShieldedSpendUnconfirmed { operation, reason: wait_err.to_string(), - }) + } } } } @@ -1330,6 +1356,73 @@ fn deserialize_note(data: &[u8]) -> Option { Note::from_parts(recipient, value, rho, rseed).into_option() } +#[cfg(test)] +mod classify_spend_wait_failure_tests { + use super::*; + use dash_sdk::error::StateTransitionBroadcastError; + use dpp::consensus::basic::decode::ProtocolVersionParsingError; + use dpp::consensus::basic::BasicError; + use dpp::consensus::ConsensusError; + + fn broadcast_err(cause: Option) -> dash_sdk::Error { + dash_sdk::Error::StateTransitionBroadcastError(StateTransitionBroadcastError { + code: 13, + message: "context deadline exceeded".to_string(), + cause, + }) + } + + #[test] + fn consensus_cause_is_a_definitive_rejection() { + let cause = ConsensusError::BasicError(BasicError::ProtocolVersionParsingError( + ProtocolVersionParsingError::new("bad version".to_string()), + )); + let err = classify_spend_wait_failure("unshield", &broadcast_err(Some(cause))); + assert!( + matches!(err, PlatformWalletError::ShieldedBroadcastFailed(_)), + "a broadcast error carrying a consensus cause means Platform ran and \ + rejected the transition; the caller may release the reservation" + ); + } + + #[test] + fn causeless_broadcast_error_is_ambiguous() { + // DAPI maps its own wait failures (e.g. `DapiError::Timeout`) to a + // `StateTransitionBroadcastError` with EMPTY consensus data, which the + // SDK decodes as `cause: None`. The spend may still execute after the + // wait gave up, so this must NOT release the note reservations. + let err = classify_spend_wait_failure("unshield", &broadcast_err(None)); + assert!( + matches!( + err, + PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "unshield", + .. + } + ), + "got {err:?}" + ); + } + + #[test] + fn transport_errors_are_ambiguous() { + let err = classify_spend_wait_failure( + "withdraw", + &dash_sdk::Error::TimeoutReached( + std::time::Duration::from_secs(80), + "waiting for response".to_string(), + ), + ); + assert!(matches!( + err, + PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "withdraw", + .. + } + )); + } +} + #[cfg(test)] mod reserve_shield_fee_tests { use super::*; diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index 04d15e31d15..21ec7d93a2c 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -30,6 +30,13 @@ public enum PlatformWalletResultCode: Int32, Sendable { /// 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 + /// A shielded spend (unshield / transfer / withdrawal) was accepted by + /// the relay but its execution result could not be confirmed. The spend + /// may have executed on chain and the wallet keeps the notes reserved + /// until the next sync (or app restart) reconciles them. Do NOT + /// auto-retry — a retry would select different notes and could + /// double-send if the original spend landed. + case errorShieldedSpendUnconfirmed = 18 case notFound = 98 case errorUnknown = 99 @@ -71,6 +78,8 @@ public enum PlatformWalletResultCode: Int32, Sendable { self = .errorShieldedBroadcastFailed case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_SHIELDED_BROADCAST_UNCONFIRMED: self = .errorShieldedBroadcastUnconfirmed + case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_SHIELDED_SPEND_UNCONFIRMED: + self = .errorShieldedSpendUnconfirmed case PLATFORM_WALLET_FFI_RESULT_CODE_NOT_FOUND: self = .notFound case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_UNKNOWN: @@ -159,6 +168,11 @@ public enum PlatformWalletError: LocalizedError { /// `outIdentityId`) before falling back to this error — see /// `ShieldedIdentityCreateUnconfirmedError`. case shieldedBroadcastUnconfirmed(String) + /// A shielded spend (unshield / transfer / withdrawal) was accepted by + /// the relay but its execution result could not be confirmed. The spend + /// may have executed; the notes stay reserved wallet-side until the next + /// sync reconciles them. Do NOT auto-retry. + case shieldedSpendUnconfirmed(String) case notFound(String) case unknown(String) @@ -173,7 +187,7 @@ public enum PlatformWalletError: LocalizedError { .serialization(let m), .deserialization(let m), .memoryAllocation(let m), .arithmeticOverflow(let m), .noSelectableInputs(let m), .walletAlreadyExists(let m), .shieldedBroadcastFailed(let m), - .shieldedBroadcastUnconfirmed(let m), + .shieldedBroadcastUnconfirmed(let m), .shieldedSpendUnconfirmed(let m), .notFound(let m), .unknown(let m): return m } @@ -203,6 +217,7 @@ public enum PlatformWalletError: LocalizedError { case .errorWalletAlreadyExists: self = .walletAlreadyExists(detail) case .errorShieldedBroadcastFailed: self = .shieldedBroadcastFailed(detail) case .errorShieldedBroadcastUnconfirmed: self = .shieldedBroadcastUnconfirmed(detail) + case .errorShieldedSpendUnconfirmed: self = .shieldedSpendUnconfirmed(detail) case .notFound: self = .notFound(detail) case .errorUnknown: self = .unknown(detail) } From 6c10203924a8d65e1b39f47f2a71b72fd59fff0f Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:18:09 +0200 Subject: [PATCH 4/8] fix(swift-sdk): treat unconfirmed shielded spends as non-retryable in the send flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Host-side follow-ups for ErrorShieldedSpendUnconfirmed (code 18): - SendViewModel catches PlatformWalletError.shieldedSpendUnconfirmed before the generic catch and surfaces it through the non-error path ("transaction may have gone through — waiting for the next shielded sync") instead of the retry-inviting error alert; the notes stay reserved Rust-side, so a retry could double-send from other notes - document the new throw on the shieldedTransfer / shieldedUnshield / shieldedWithdraw wrappers - add a unit test pinning map_spend_result's retry-relevant code split Co-Authored-By: Claude Fable 5 --- .../src/shielded_send.rs | 66 +++++++++++++++++++ .../PlatformWalletManagerShieldedSync.swift | 18 +++++ .../Core/ViewModels/SendViewModel.swift | 10 +++ 3 files changed, 94 insertions(+) diff --git a/packages/rs-platform-wallet-ffi/src/shielded_send.rs b/packages/rs-platform-wallet-ffi/src/shielded_send.rs index 95269402d62..d2ec755db7b 100644 --- a/packages/rs-platform-wallet-ffi/src/shielded_send.rs +++ b/packages/rs-platform-wallet-ffi/src/shielded_send.rs @@ -1154,4 +1154,70 @@ mod tests { ); } } + + /// Read the Rust-owned message out of an FFI result for assertions. + fn message_of(result: &PlatformWalletFFIResult) -> String { + assert!( + !result.message.is_null(), + "error result must carry a message" + ); + unsafe { CStr::from_ptr(result.message) } + .to_string_lossy() + .into_owned() + } + + /// `map_spend_result` pins the retry-relevant code split the three spend + /// entry points depend on: + /// - `ShieldedSpendUnconfirmed` → `ErrorShieldedSpendUnconfirmed` (host + /// must NOT retry — the notes stay reserved; a retry could select other + /// unreserved notes and double-send), + /// - `ShieldedBroadcastFailed` → `ErrorShieldedBroadcastFailed` + /// (definitive failure; reservations released; safe to retry), + /// - any other variant → the generic `ErrorWalletOperation`. + /// + /// The typed `Display` rendering must survive into the result message in + /// every error arm so callers keep diagnostics across the boundary. + #[test] + fn map_spend_result_pins_retry_relevant_codes() { + let unconfirmed: Result<(), PlatformWalletError> = + Err(PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "unshield", + reason: "transient proof fetch failed".to_string(), + }); + let result = map_spend_result(unconfirmed, "shielded unshield"); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorShieldedSpendUnconfirmed + ); + assert!( + message_of(&result).contains("transient proof fetch failed"), + "unconfirmed message must carry the wallet Display payload" + ); + + let failed: Result<(), PlatformWalletError> = Err( + PlatformWalletError::ShieldedBroadcastFailed("relay rejected".to_string()), + ); + let result = map_spend_result(failed, "shielded transfer"); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorShieldedBroadcastFailed + ); + assert!( + message_of(&result).contains("relay rejected"), + "broadcast-failed message must carry the wallet Display payload" + ); + + let other: Result<(), PlatformWalletError> = + Err(PlatformWalletError::ShieldedNoUnspentNotes); + let result = map_spend_result(other, "shielded withdraw"); + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorWalletOperation + ); + + assert_eq!( + map_spend_result(Ok(()), "shielded transfer").code, + PlatformWalletFFIResultCode::Success + ); + } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift index 8fb35da1f5c..501a29a7155 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift @@ -491,6 +491,12 @@ extension PlatformWalletManager { /// a non-empty memo's UTF-8 byte length must be at most 32 or /// Rust rejects it. The 36-byte on-chain encoding is done on the /// Rust side. + /// + /// Throws `PlatformWalletError.shieldedSpendUnconfirmed` when the + /// broadcast was accepted but its execution result couldn't be + /// confirmed — the spend may already be on chain, so the caller + /// must NOT retry (the spent notes stay reserved Rust-side; the + /// next shielded sync reconciles them). public func shieldedTransfer( walletId: Data, account: UInt32 = 0, @@ -607,6 +613,12 @@ extension PlatformWalletManager { /// string (`"dash1…"` on mainnet, `"tdash1…"` on testnet). Rust /// parses and network-checks the address; hosts don't have to /// hand-roll the bincode storage variant tag. + /// + /// Throws `PlatformWalletError.shieldedSpendUnconfirmed` when the + /// broadcast was accepted but its execution result couldn't be + /// confirmed — the spend may already be on chain, so the caller + /// must NOT retry (the spent notes stay reserved Rust-side; the + /// next shielded sync reconciles them). public func shieldedUnshield( walletId: Data, account: UInt32 = 0, @@ -649,6 +661,12 @@ extension PlatformWalletManager { /// shielded balance and creates an L1 withdrawal to /// `toCoreAddress` (Base58Check string). `coreFeePerByte` is /// the L1 fee rate in duffs/byte (`1` is the dashmate default). + /// + /// Throws `PlatformWalletError.shieldedSpendUnconfirmed` when the + /// broadcast was accepted but its execution result couldn't be + /// confirmed — the spend may already be on chain, so the caller + /// must NOT retry (the spent notes stay reserved Rust-side; the + /// next shielded sync reconciles them). public func shieldedWithdraw( walletId: Data, account: UInt32 = 0, diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift index 86703a5517d..45ecece47a1 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift @@ -499,6 +499,16 @@ class SendViewModel: ObservableObject { successMessage = "Shielding complete" } + } catch PlatformWalletError.shieldedSpendUnconfirmed { + // The shielded spend (unshield / transfer / withdraw) was broadcast + // and accepted, but its execution result couldn't be confirmed — it + // may already be on chain. Rust intentionally KEEPS the spent notes' + // reservations, so this must NOT be presented as a retryable failure: + // retrying would select other unreserved notes and double-send the + // payment. Surface it through the non-error (success) path so the UI + // doesn't invite a retry; the next shielded sync reconciles the notes. + successMessage = "Transaction may have gone through — waiting for " + + "the next shielded sync to confirm. Do not retry." } catch { self.error = error.localizedDescription } From c38b99de00e8631f18815c5aac56e721868a22b5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:27:36 +0200 Subject: [PATCH 5/8] fix(platform-wallet): treat ambiguous shielded broadcast() failures as unconfirmed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit broadcast() runs through the dapi-client retry machinery, so a transport error or timeout cannot prove the transition never reached a node — the request may have been delivered with only the ACK lost, and AlreadyExists proves the opposite (the transition is already in the mempool or on chain). Mapping every broadcast() error to ShieldedBroadcastFailed released the note reservations in those unknown-outcome cases, letting a host retry select other unreserved notes and double-send if the original broadcast landed. Only a consensus verdict is definitive at the broadcast stage: a CheckTx rejection arrives as Error::Protocol(ConsensusError) (DAPI re-attaches the serialized consensus error as gRPC metadata, which the dapi-client decodes), so genuine rejections still release the notes and stay retryable. Everything else now classifies as ShieldedSpendUnconfirmed for spends and ShieldedBroadcastUnconfirmed (with the derived id, holding the slot) for identity-create — the same recovery path as a wait-stage ambiguity. The shared carries_consensus_rejection predicate also broadens the wait-stage classifier to accept the consensus-metadata shape as a verdict. Tests cover consensus/transport/AlreadyExists classification at both stages. Co-Authored-By: Claude Fable 5 --- .../src/wallet/shielded/operations.rs | 214 +++++++++++++++--- 1 file changed, 179 insertions(+), 35 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 0fe23d03cd2..233c041d656 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -789,12 +789,31 @@ where ) .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()))?; + // Broadcast (relay-ACK only). Only a consensus-verdict rejection is definitive here — + // CheckTx evaluated the transition and refused it, so it sits in no mempool and the outer + // match may release the reservation via `ShieldedBroadcastFailed`, exactly as before. Any + // other failure leaves the outcome unknown: the dapi-client retries the broadcast across + // addresses, so a transport error or timeout cannot prove the request never reached a node + // (the ACK may simply have been lost), and `AlreadyExists` proves the transition IS already + // in the mempool or on chain. Surface those as `ShieldedBroadcastUnconfirmed` with the + // derived id so the caller holds the slot and keeps the note reservations — the same + // recovery path as a wait-stage ambiguity (the next sync reconciles). + st.broadcast(sdk, None).await.map_err(|e| { + if carries_consensus_rejection(&e) { + PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) + } else { + warn!( + derived_id = %identity_id, + error = %e, + "IdentityCreateFromShieldedPool: broadcast outcome unknown (no consensus \ + rejection); holding the slot and keeping the note reservations" + ); + PlatformWalletError::ShieldedBroadcastUnconfirmed { + identity_id, + reason: e.to_string(), + } + } + })?; // Wait for proven execution. Classify the failure: // - `StateTransitionBroadcastError` WITH a consensus `cause` = Platform DEFINITIVELY @@ -1225,20 +1244,47 @@ async fn cancel_pending( } } +/// Whether an SDK error carries Platform's own consensus verdict on the +/// transition. Two shapes qualify: +/// +/// - `Error::Protocol(ProtocolError::ConsensusError(_))` — DAPI attached the +/// serialized consensus error as gRPC metadata +/// (`dash-serialized-consensus-error-bin`), which the dapi-client decodes +/// on any failed request. This is how a CheckTx rejection of the +/// transition surfaces from `broadcast()` (rs-dapi's +/// `map_broadcast_error` decodes the consensus error from Tenderdash's +/// `info` field and `TenderdashStatus` re-attaches it as metadata); +/// - a `StateTransitionBroadcastError` whose `cause` deserialized from +/// non-empty consensus `data` — the wait-stream error envelope for a +/// transition Platform executed and rejected on its merits. +/// +/// Only these prove the transition was evaluated and REJECTED. Everything +/// else — transport errors, timeouts, `AlreadyExists` (which proves the +/// opposite: the transition is already in the mempool or on chain), +/// DAPI-internal failures, cause-less broadcast envelopes (the shape DAPI +/// uses for its own wait-side timeouts) — leaves the outcome unknown. +fn carries_consensus_rejection(err: &dash_sdk::Error) -> bool { + match err { + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(_)) => true, + dash_sdk::Error::StateTransitionBroadcastError(e) => e.cause.is_some(), + _ => false, + } +} + /// Broadcast a built shielded spend transition (unshield / transfer / /// withdraw) and wait for proven execution, staging the two SDK calls /// separately so the caller's reservation rollback only runs when the /// spend DEFINITIVELY did not happen: /// -/// - a broadcast-time rejection (relay/CheckTx refused the tx), or a -/// `StateTransitionBroadcastError` carrying a consensus `cause` -/// (Platform ran the transition and rejected it on its merits), means -/// the spend never executed → -/// [`PlatformWalletError::ShieldedBroadcastFailed`], and the caller -/// releases the note reservations via [`cancel_pending`]; -/// - any other wait failure (result-proof fetch/verify error, timeout, -/// transport error, …) is AMBIGUOUS — the relay accepted the tx and -/// it may well have executed → +/// - a consensus-verdict rejection at either stage (CheckTx refused the +/// broadcast, or Platform ran the transition and rejected it on its +/// merits — see [`carries_consensus_rejection`]) means the spend never +/// executed → [`PlatformWalletError::ShieldedBroadcastFailed`], and the +/// caller releases the note reservations via [`cancel_pending`]; +/// - any other failure at either stage (transport error, timeout, +/// `AlreadyExists`, result-proof fetch/verify error, …) is AMBIGUOUS — +/// the dapi-client retries across addresses, so even a failed +/// `broadcast()` call may have delivered the transition to a node → /// [`PlatformWalletError::ShieldedSpendUnconfirmed`], and the caller /// must leave the reservations in place (each spend flow's outer /// match has a dedicated arm for this). @@ -1256,7 +1302,7 @@ async fn broadcast_shielded_spend( state_transition .broadcast(sdk, None) .await - .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; + .map_err(|broadcast_err| classify_spend_broadcast_failure(operation, &broadcast_err))?; state_transition .wait_for_response::(sdk, None) @@ -1265,13 +1311,46 @@ async fn broadcast_shielded_spend( .map_err(|wait_err| classify_spend_wait_failure(operation, &wait_err)) } +/// Classify a `broadcast()` failure for a shielded spend — the +/// broadcast-stage sibling of [`classify_spend_wait_failure`]. +/// +/// Only a consensus verdict ([`carries_consensus_rejection`]) is +/// definitive at this stage: CheckTx evaluated the transition and refused +/// it, so it sits in no mempool and the reservations may be released. +/// Anything else is ambiguous even before the wait: the dapi-client +/// retries the broadcast across addresses, so a transport failure or +/// timeout cannot prove the request never reached a node (the ACK may +/// simply have been lost), and `AlreadyExists` proves the transition IS +/// already in the mempool or on chain. Keep the reservations; the next +/// nullifier sync (or an app restart, since reservations are in-memory) +/// reconciles them. +fn classify_spend_broadcast_failure( + operation: &'static str, + broadcast_err: &dash_sdk::Error, +) -> PlatformWalletError { + if carries_consensus_rejection(broadcast_err) { + PlatformWalletError::ShieldedBroadcastFailed(broadcast_err.to_string()) + } else { + warn!( + operation, + error = %broadcast_err, + "Shielded spend broadcast outcome unknown (no consensus rejection); \ + leaving the note reservations in place" + ); + PlatformWalletError::ShieldedSpendUnconfirmed { + operation, + reason: broadcast_err.to_string(), + } + } +} + /// Classify a `wait_for_response` failure for an already-broadcast /// shielded spend (see [`broadcast_shielded_spend`]). /// -/// Only a `StateTransitionBroadcastError` with a populated consensus -/// `cause` proves Platform executed the transition and rejected it on -/// its merits — the serialized consensus error is the verdict. DAPI -/// encodes its own wait-side failures (timeouts, internal errors; see +/// Only a consensus verdict ([`carries_consensus_rejection`]) proves +/// Platform executed the transition and rejected it on its merits — the +/// serialized consensus error is the verdict. DAPI encodes its own +/// wait-side failures (timeouts, internal errors; see /// `build_wait_for_state_transition_error_response` in rs-dapi) as /// `StateTransitionBroadcastError`s with EMPTY consensus data, which /// the SDK surfaces as `cause: None` — those are ambiguous, not @@ -1284,21 +1363,18 @@ fn classify_spend_wait_failure( operation: &'static str, wait_err: &dash_sdk::Error, ) -> PlatformWalletError { - match wait_err { - dash_sdk::Error::StateTransitionBroadcastError(e) if e.cause.is_some() => { - PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) - } - _ => { - warn!( - operation, - error = %wait_err, - "Shielded spend broadcast accepted but result confirmation failed; \ - leaving the note reservations in place" - ); - PlatformWalletError::ShieldedSpendUnconfirmed { - operation, - reason: wait_err.to_string(), - } + if carries_consensus_rejection(wait_err) { + PlatformWalletError::ShieldedBroadcastFailed(wait_err.to_string()) + } else { + warn!( + operation, + error = %wait_err, + "Shielded spend broadcast accepted but result confirmation failed; \ + leaving the note reservations in place" + ); + PlatformWalletError::ShieldedSpendUnconfirmed { + operation, + reason: wait_err.to_string(), } } } @@ -1421,6 +1497,74 @@ mod classify_spend_wait_failure_tests { } )); } + + /// The shape a CheckTx rejection takes when it reaches the SDK from + /// `broadcast()`: DAPI re-attaches the serialized consensus error as + /// gRPC metadata and the dapi-client decodes it into + /// `Error::Protocol(ConsensusError)`. + fn consensus_metadata_rejection() -> dash_sdk::Error { + let cause = ConsensusError::BasicError(BasicError::ProtocolVersionParsingError( + ProtocolVersionParsingError::new("bad version".to_string()), + )); + dash_sdk::Error::Protocol(dpp::ProtocolError::ConsensusError(Box::new(cause))) + } + + /// A CheckTx consensus rejection surfacing from `broadcast()` IS + /// definitive: the transition was evaluated and refused, it sits in no + /// mempool, and the caller may release the reservations and retry. + #[test] + fn broadcast_consensus_rejection_is_definitive() { + let err = classify_spend_broadcast_failure("transfer", &consensus_metadata_rejection()); + assert!(matches!( + err, + PlatformWalletError::ShieldedBroadcastFailed(_) + )); + // The same verdict shape is definitive on the wait stage too. + let err = classify_spend_wait_failure("transfer", &consensus_metadata_rejection()); + assert!(matches!( + err, + PlatformWalletError::ShieldedBroadcastFailed(_) + )); + } + + /// A transport/timeout failure of the `broadcast()` call itself is + /// AMBIGUOUS: the dapi-client retries across addresses, so the request + /// may have been delivered to a node whose ACK was lost. Releasing the + /// reservations here would let a retry select other unreserved notes + /// and double-send if the original broadcast landed. + #[test] + fn broadcast_transport_failure_is_ambiguous() { + let err = classify_spend_broadcast_failure( + "unshield", + &dash_sdk::Error::Generic("transport error: connection reset".to_string()), + ); + assert!(matches!( + err, + PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "unshield", + .. + } + )); + } + + /// `AlreadyExists` from `broadcast()` proves the transition IS already + /// in the mempool or on chain (e.g. an internal dapi-client retry after + /// an ambiguous first delivery), so it must NOT release the + /// reservations — the spend is in flight. + #[test] + fn broadcast_already_exists_is_ambiguous() { + let err = classify_spend_broadcast_failure( + "withdraw", + &dash_sdk::Error::AlreadyExists("state transition already in mempool".to_string()), + ); + assert!(matches!( + err, + PlatformWalletError::ShieldedSpendUnconfirmed { + operation: "withdraw", + .. + } + )); + } } #[cfg(test)] From 1fe849c281f2500f917996ec8c05a0bf0be4f6bc Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:27:37 +0200 Subject: [PATCH 6/8] fix(swift-example-app): hold unconfirmed Type-20 slot reservations across controller teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit usedIdentityIndices(for:) was built only from PersistentIdentity rows, but an .unconfirmed Type-20 registration has no identity row until the next sync confirms it — so the markIdentitySlotUsed reservation evaporated with the in-memory controller and the same identityIndex became selectable again, re-opening the duplicate-submission path the .unconfirmed state exists to prevent. Union the persisted PersistentCoreAddress.isUsed reservations on the identity-registration account into the selection source of truth; over-reporting only skips an index (registration indices aren't gap-limited) while identity rows remain authoritative for confirmed identities. Also broaden the Swift code-16 docs (errorShieldedBroadcastFailed / shieldedBroadcastFailed) to cover the spend flows that now map to it, not just Type-20. Co-Authored-By: Claude Fable 5 --- .../PlatformWallet/PlatformWalletResult.swift | 12 +++-- .../Views/CreateIdentityView.swift | 51 +++++++++++++------ 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift index 21ec7d93a2c..453cfd1f2de 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -21,9 +21,10 @@ 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. + /// Definitive shielded-broadcast failure: the shielded transition + /// (identity-create, unshield, transfer, or withdrawal) 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 @@ -158,8 +159,9 @@ 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. + /// Definitive shielded-broadcast failure: the shielded transition + /// (identity-create or a spend — unshield / transfer / withdrawal) 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 diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift index dd1f4619210..14cdf71e663 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift @@ -1608,9 +1608,14 @@ struct CreateIdentityView: View { } /// Flip `isUsed` on the consumed identity-registration slot so - /// the next call to `unusedIdentityIndices` skips it. Silent - /// no-op if the slot isn't found — this is cosmetic bookkeeping - /// and the Rust side is already the source of truth. + /// `usedIdentityIndices` (which unions the flag with the + /// `PersistentIdentity` rows) skips it. For confirmed registrations + /// this is redundant with the identity row, but for `.unconfirmed` + /// Type-20 broadcasts it is the ONLY persisted reservation holding + /// the slot until the next sync writes the identity row — losing it + /// would offer the same index for a duplicate submission. Silent + /// no-op if the slot row isn't found (an index beyond the derived + /// lookahead); the Rust side remains the on-chain source of truth. private func markIdentitySlotUsed( walletId: Data, identityIndex: UInt32 @@ -2001,23 +2006,39 @@ struct CreateIdentityView: View { } } - /// Unused identity-registration key indices on the wallet's - /// Identity Registration account (FFI type tag 2). Each - /// `PersistentCoreAddress` under that account represents one - /// registration slot keyed by `addressIndex`; `isUsed` flips to - /// true once the slot has been consumed by a prior identity - /// creation. Returns an ascending list of the remaining slots. - /// Identity-registration indices currently claimed by an existing - /// `PersistentIdentity` on this wallet. Single source of truth — - /// the deprecated `PersistentCoreAddress.isUsed` flag was a - /// denormalized cache that drifted (discovered identities never - /// flipped it). + /// Identity-registration indices that must not be offered for a new + /// registration on this wallet: every index claimed by an existing + /// `PersistentIdentity`, UNIONED with the persisted + /// `PersistentCoreAddress.isUsed` reservations on the + /// identity-registration account. + /// + /// The union matters for `.unconfirmed` Type-20 registrations: the + /// broadcast landed but no `PersistentIdentity` row exists until the + /// next sync confirms the identity, so `markIdentitySlotUsed`'s + /// persisted `isUsed` flag is the ONLY artifact holding the slot once + /// the in-memory controller is gone. Without it the same index would + /// be offered again and a duplicate submission would burn funds + /// against the registered-key-hash stateful check. + /// + /// Identity rows stay authoritative for confirmed identities; the flag + /// is monotonic extra coverage. Its historical drift (discovered + /// identities never flipped it) only under-reports, which the identity + /// rows cover, and over-reporting merely skips an index — registration + /// indices aren't gap-limited, so a skipped index costs nothing. private func usedIdentityIndices(for walletId: Data) -> Set { - Set( + var used = Set( allIdentities .filter { $0.wallet?.walletId == walletId } .map { $0.identityIndex } ) + if let account = identityRegistrationAccount(for: walletId) { + used.formUnion( + account.coreAddresses + .filter(\.isUsed) + .map(\.addressIndex) + ) + } + return used } /// One past the highest used registration index on this wallet, From d0887c1071328c0ec9818680d1f9b7503476e2dd Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 05:35:17 +0200 Subject: [PATCH 7/8] fix(platform-wallet): confirm no-verdict broadcasts via the result wait instead of failing fast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refine the broadcast-stage disposition from c38b99de00, which classified every non-consensus broadcast() failure as unconfirmed immediately. Two problems with that: AlreadyExists (the tx IS in the mempool after a lost-ACK retry hit tenderdash's dedupe) was reported as "unconfirmed" when one wait_for_response away from a real confirmation — and for identity-create it skipped the fetch-by-derived-id fallback entirely; and the common offline failure (connect refused → gRPC Unavailable) stranded the note reservations until app restart even though nothing was ever delivered. Replace classify_spend_broadcast_failure with a three-way disposition: - definitive (broadcast_definitely_failed): consensus-verdict CheckTx rejections, server admission-refusals (InvalidArgument, ResourceExhausted = mempool full, …), connection-establishment failures (Unavailable, NoAvailableAddresses) → ShieldedBroadcastFailed and the reservations release, keeping offline sends immediately retryable; - no-verdict (AlreadyExists, TimeoutReached, Cancelled, gRPC DeadlineExceeded/Internal/Unknown/Aborted/DataLoss — shapes that can postdate delivery, including as the terminal retry error) → fall through to wait_for_response, which proves execution, returns a consensus rejection (definitive after all), or classifies the residual ambiguity as unconfirmed — for identity-create via the existing fetch-by-derived-id fallback. The wait-stage classification and carries_consensus_rejection are unchanged. Tests updated to pin the disposition table. Co-Authored-By: Claude Fable 5 --- .../src/wallet/shielded/operations.rs | 253 +++++++++++------- 1 file changed, 158 insertions(+), 95 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 233c041d656..2b6f174ce03 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -789,31 +789,29 @@ where ) .map_err(|e| PlatformWalletError::ShieldedBuildError(e.to_string()))?; - // Broadcast (relay-ACK only). Only a consensus-verdict rejection is definitive here — - // CheckTx evaluated the transition and refused it, so it sits in no mempool and the outer - // match may release the reservation via `ShieldedBroadcastFailed`, exactly as before. Any - // other failure leaves the outcome unknown: the dapi-client retries the broadcast across - // addresses, so a transport error or timeout cannot prove the request never reached a node - // (the ACK may simply have been lost), and `AlreadyExists` proves the transition IS already - // in the mempool or on chain. Surface those as `ShieldedBroadcastUnconfirmed` with the - // derived id so the caller holds the slot and keeps the note reservations — the same - // recovery path as a wait-stage ambiguity (the next sync reconciles). - st.broadcast(sdk, None).await.map_err(|e| { - if carries_consensus_rejection(&e) { - PlatformWalletError::ShieldedBroadcastFailed(e.to_string()) - } else { + // Broadcast (relay-ACK only). Definitive failures — a consensus-verdict CheckTx + // rejection, or a transport failure that proves nothing was delivered (see + // `broadcast_definitely_failed`; connect-refused/offline is the common case) — map to + // `ShieldedBroadcastFailed` and let the outer match release the reservation, exactly as + // before. No-verdict failures (`AlreadyExists` proves the transition IS already in the + // mempool or on chain after a lost-ACK retry; timeouts merely allow it) fall through to + // the result wait below, whose ambiguous arm already owns the fetch-by-derived-id + // fallback — so the in-flight tx gets confirmed (or held as unconfirmed) instead of + // being reported as failed. + match st.broadcast(sdk, None).await { + Ok(()) => {} + Err(e) if broadcast_definitely_failed(&e) => { + return Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())); + } + Err(e) => { warn!( derived_id = %identity_id, error = %e, - "IdentityCreateFromShieldedPool: broadcast outcome unknown (no consensus \ - rejection); holding the slot and keeping the note reservations" + "IdentityCreateFromShieldedPool: broadcast returned no verdict; the \ + transition may have been admitted — falling through to the result wait" ); - PlatformWalletError::ShieldedBroadcastUnconfirmed { - identity_id, - reason: e.to_string(), - } } - })?; + } // Wait for proven execution. Classify the failure: // - `StateTransitionBroadcastError` WITH a consensus `cause` = Platform DEFINITIVELY @@ -1276,15 +1274,20 @@ fn carries_consensus_rejection(err: &dash_sdk::Error) -> bool { /// separately so the caller's reservation rollback only runs when the /// spend DEFINITIVELY did not happen: /// -/// - a consensus-verdict rejection at either stage (CheckTx refused the -/// broadcast, or Platform ran the transition and rejected it on its -/// merits — see [`carries_consensus_rejection`]) means the spend never -/// executed → [`PlatformWalletError::ShieldedBroadcastFailed`], and the -/// caller releases the note reservations via [`cancel_pending`]; -/// - any other failure at either stage (transport error, timeout, -/// `AlreadyExists`, result-proof fetch/verify error, …) is AMBIGUOUS — -/// the dapi-client retries across addresses, so even a failed -/// `broadcast()` call may have delivered the transition to a node → +/// - a definitive `broadcast()` failure ([`broadcast_definitely_failed`]: +/// a consensus-verdict CheckTx rejection, or a transport failure that +/// proves nothing was delivered), or a wait-stage consensus rejection +/// (Platform ran the transition and rejected it on its merits — see +/// [`carries_consensus_rejection`]), means the spend never executed → +/// [`PlatformWalletError::ShieldedBroadcastFailed`], and the caller +/// releases the note reservations via [`cancel_pending`]; +/// - a no-verdict `broadcast()` failure (`AlreadyExists` proves the tx IS +/// in the mempool after a lost-ACK retry; timeouts merely allow it) +/// falls through to the result wait, which either proves execution or +/// classifies the residual ambiguity; +/// - any other wait failure (transport error, timeout, result-proof +/// fetch/verify error, …) is AMBIGUOUS — the relay accepted (or may +/// have accepted) the tx and it may well have executed → /// [`PlatformWalletError::ShieldedSpendUnconfirmed`], and the caller /// must leave the reservations in place (each spend flow's outer /// match has a dedicated arm for this). @@ -1299,10 +1302,20 @@ async fn broadcast_shielded_spend( state_transition: &StateTransition, operation: &'static str, ) -> Result<(), PlatformWalletError> { - state_transition - .broadcast(sdk, None) - .await - .map_err(|broadcast_err| classify_spend_broadcast_failure(operation, &broadcast_err))?; + match state_transition.broadcast(sdk, None).await { + Ok(()) => {} + Err(e) if broadcast_definitely_failed(&e) => { + return Err(PlatformWalletError::ShieldedBroadcastFailed(e.to_string())); + } + Err(e) => { + warn!( + operation, + error = %e, + "Shielded spend broadcast returned no verdict; the transition may have been \ + admitted — falling through to the result wait" + ); + } + } state_transition .wait_for_response::(sdk, None) @@ -1311,36 +1324,60 @@ async fn broadcast_shielded_spend( .map_err(|wait_err| classify_spend_wait_failure(operation, &wait_err)) } -/// Classify a `broadcast()` failure for a shielded spend — the -/// broadcast-stage sibling of [`classify_spend_wait_failure`]. +/// Whether a failed `broadcast()` call DEFINITIVELY left the transition +/// out of every mempool, so any note reservations may be released and the +/// caller may rebuild and retry: /// -/// Only a consensus verdict ([`carries_consensus_rejection`]) is -/// definitive at this stage: CheckTx evaluated the transition and refused -/// it, so it sits in no mempool and the reservations may be released. -/// Anything else is ambiguous even before the wait: the dapi-client -/// retries the broadcast across addresses, so a transport failure or -/// timeout cannot prove the request never reached a node (the ACK may -/// simply have been lost), and `AlreadyExists` proves the transition IS -/// already in the mempool or on chain. Keep the reservations; the next -/// nullifier sync (or an app restart, since reservations are in-memory) -/// reconciles them. -fn classify_spend_broadcast_failure( - operation: &'static str, - broadcast_err: &dash_sdk::Error, -) -> PlatformWalletError { - if carries_consensus_rejection(broadcast_err) { - PlatformWalletError::ShieldedBroadcastFailed(broadcast_err.to_string()) - } else { - warn!( - operation, - error = %broadcast_err, - "Shielded spend broadcast outcome unknown (no consensus rejection); \ - leaving the note reservations in place" - ); - PlatformWalletError::ShieldedSpendUnconfirmed { - operation, - reason: broadcast_err.to_string(), +/// - a consensus verdict ([`carries_consensus_rejection`]): CheckTx +/// evaluated the transition and refused it; +/// - a gRPC response whose status code is a server-side rejection or a +/// connection-establishment failure. `Unavailable` is the shape every +/// connect-refused/offline attempt takes — classifying it as definitive +/// keeps the common no-network failure's notes immediately re-spendable +/// instead of stranding them until the next restart — and rejection +/// codes (`InvalidArgument`, `ResourceExhausted` = mempool full, …) are +/// verdicts that the tx was refused admission; +/// - no usable DAPI addresses at all (nothing was ever sent). +/// +/// Everything else leaves the outcome unknown and the caller must fall +/// through to the result wait instead of failing: `AlreadyExists` proves +/// the tx IS in the mempool or on chain (a lost-ACK attempt was re-sent +/// by the dapi-client retry and hit tenderdash's dedupe), and +/// timeout/cancellation/no-response shapes (`TimeoutReached`, +/// `Cancelled`, gRPC `DeadlineExceeded`/`Cancelled`, plus +/// `Internal`/`Unknown`/`Aborted`/`DataLoss`, which DAPI also uses for +/// its own tenderdash-side failures that can postdate delivery) allow +/// the request to have outlived its lost ACK. +fn broadcast_definitely_failed(e: &dash_sdk::Error) -> bool { + use dash_sdk::dapi_client::transport::TransportError; + use dash_sdk::dapi_client::DapiClientError; + use dash_sdk::dapi_grpc::tonic::Code; + + fn status_is_verdict(t: &TransportError) -> bool { + let TransportError::Grpc(status) = t; + !matches!( + status.code(), + Code::DeadlineExceeded + | Code::Cancelled + | Code::Unknown + | Code::Internal + | Code::Aborted + | Code::DataLoss + ) + } + + if carries_consensus_rejection(e) { + return true; + } + match e { + dash_sdk::Error::AlreadyExists(_) => false, + dash_sdk::Error::DapiClientError(DapiClientError::Transport(t)) => status_is_verdict(t), + dash_sdk::Error::DapiClientError(DapiClientError::NoAvailableAddresses) => true, + dash_sdk::Error::DapiClientError(DapiClientError::NoAvailableAddressesToRetry(t)) => { + status_is_verdict(t) } + dash_sdk::Error::NoAvailableAddressesToRetry(inner) => broadcast_definitely_failed(inner), + _ => false, } } @@ -1514,11 +1551,7 @@ mod classify_spend_wait_failure_tests { /// mempool, and the caller may release the reservations and retry. #[test] fn broadcast_consensus_rejection_is_definitive() { - let err = classify_spend_broadcast_failure("transfer", &consensus_metadata_rejection()); - assert!(matches!( - err, - PlatformWalletError::ShieldedBroadcastFailed(_) - )); + assert!(broadcast_definitely_failed(&consensus_metadata_rejection())); // The same verdict shape is definitive on the wait stage too. let err = classify_spend_wait_failure("transfer", &consensus_metadata_rejection()); assert!(matches!( @@ -1527,42 +1560,72 @@ mod classify_spend_wait_failure_tests { )); } - /// A transport/timeout failure of the `broadcast()` call itself is - /// AMBIGUOUS: the dapi-client retries across addresses, so the request - /// may have been delivered to a node whose ACK was lost. Releasing the - /// reservations here would let a retry select other unreserved notes - /// and double-send if the original broadcast landed. + fn grpc_err(code: dash_sdk::dapi_grpc::tonic::Code) -> dash_sdk::Error { + use dash_sdk::dapi_client::transport::TransportError; + use dash_sdk::dapi_client::DapiClientError; + dash_sdk::Error::DapiClientError(DapiClientError::Transport(TransportError::Grpc( + dash_sdk::dapi_grpc::tonic::Status::new(code, "boom"), + ))) + } + + /// Connection-establishment failures and non-consensus server + /// rejections of `broadcast()` ARE definitive: nothing was admitted to + /// a mempool. The offline case (connect refused → `Unavailable`) in + /// particular must release the notes immediately rather than strand + /// them until restart. #[test] - fn broadcast_transport_failure_is_ambiguous() { - let err = classify_spend_broadcast_failure( - "unshield", - &dash_sdk::Error::Generic("transport error: connection reset".to_string()), - ); - assert!(matches!( - err, - PlatformWalletError::ShieldedSpendUnconfirmed { - operation: "unshield", - .. - } + fn broadcast_connection_failures_and_rejections_are_definitive() { + use dash_sdk::dapi_grpc::tonic::Code; + assert!(broadcast_definitely_failed(&grpc_err(Code::Unavailable))); + // Mempool full / malformed request: server verdicts refusing admission. + assert!(broadcast_definitely_failed(&grpc_err( + Code::ResourceExhausted + ))); + assert!(broadcast_definitely_failed(&grpc_err( + Code::InvalidArgument + ))); + } + + /// No-response shapes of the `broadcast()` call itself are NOT + /// definitive: the dapi-client retries across addresses, so the request + /// may have been delivered to a node whose ACK was lost. The caller + /// falls through to the result wait instead of releasing the + /// reservations (which would let a retry select other unreserved notes + /// and double-send if the original broadcast landed). + #[test] + fn broadcast_no_response_shapes_are_inconclusive() { + use dash_sdk::dapi_grpc::tonic::Code; + assert!(!broadcast_definitely_failed(&grpc_err( + Code::DeadlineExceeded + ))); + // DAPI maps its own tenderdash-side failures (which can postdate + // delivery) to Internal. + assert!(!broadcast_definitely_failed(&grpc_err(Code::Internal))); + assert!(!broadcast_definitely_failed( + &dash_sdk::Error::TimeoutReached( + std::time::Duration::from_secs(30), + "broadcast".to_string(), + ) + )); + assert!(!broadcast_definitely_failed(&dash_sdk::Error::Generic( + "transport error: connection reset".to_string() + ))); + // …including as the terminal error of an exhausted retry loop. + assert!(!broadcast_definitely_failed( + &dash_sdk::Error::NoAvailableAddressesToRetry(Box::new(grpc_err( + Code::DeadlineExceeded + ))) )); } /// `AlreadyExists` from `broadcast()` proves the transition IS already /// in the mempool or on chain (e.g. an internal dapi-client retry after - /// an ambiguous first delivery), so it must NOT release the - /// reservations — the spend is in flight. + /// an ambiguous first delivery), so it must NOT be treated as a failure + /// — the spend is in flight and the result wait will confirm it. #[test] - fn broadcast_already_exists_is_ambiguous() { - let err = classify_spend_broadcast_failure( - "withdraw", - &dash_sdk::Error::AlreadyExists("state transition already in mempool".to_string()), - ); - assert!(matches!( - err, - PlatformWalletError::ShieldedSpendUnconfirmed { - operation: "withdraw", - .. - } + fn broadcast_already_exists_is_in_flight() { + assert!(!broadcast_definitely_failed( + &dash_sdk::Error::AlreadyExists("state transition already in mempool".to_string()) )); } } From 231f22b92825b66947dc387cd55a8af81921a68e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 12 Jun 2026 06:09:34 +0200 Subject: [PATCH 8/8] docs(platform-wallet): qualify the Unavailable-is-definitive claim in broadcast_definitely_failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review nitpick on #3863: Unavailable is the common connect-refused shape but not an absolute never-delivered guarantee — HTTP/2 stream resets after the request bytes left surface the same code, and the dapi-client cross-address retry only retains the last transport error. Document the residual window and cross-reference the unshield finalize_pending rationale showing it is fund-safe (on-chain nullifier set; worst case a wasted proof), framing the classification as a deliberate UX trade. Co-Authored-By: Claude Fable 5 --- .../src/wallet/shielded/operations.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs index 2b6f174ce03..cad0c71c86a 100644 --- a/packages/rs-platform-wallet/src/wallet/shielded/operations.rs +++ b/packages/rs-platform-wallet/src/wallet/shielded/operations.rs @@ -1331,14 +1331,26 @@ async fn broadcast_shielded_spend( /// - a consensus verdict ([`carries_consensus_rejection`]): CheckTx /// evaluated the transition and refused it; /// - a gRPC response whose status code is a server-side rejection or a -/// connection-establishment failure. `Unavailable` is the shape every -/// connect-refused/offline attempt takes — classifying it as definitive -/// keeps the common no-network failure's notes immediately re-spendable +/// connection-establishment failure. `Unavailable` is the common shape +/// of a connect-refused/offline attempt — classifying it as definitive +/// keeps the no-network failure's notes immediately re-spendable /// instead of stranding them until the next restart — and rejection /// codes (`InvalidArgument`, `ResourceExhausted` = mempool full, …) are /// verdicts that the tx was refused admission; /// - no usable DAPI addresses at all (nothing was ever sent). /// +/// `Unavailable` is NOT an absolute never-delivered guarantee: HTTP/2 +/// stream resets after the request bytes left can surface the same code, +/// and the dapi-client's cross-address retry only retains the LAST +/// transport error, so an earlier-attempt delivery can hide behind a +/// later attempt's `Unavailable`. Releasing the notes in that residual +/// window is still fund-safe — the authoritative no-reuse guarantee is +/// the on-chain nullifier set, so a re-selected note at worst wastes a +/// ~30 s proof on a nullifier-already-used rejection (see the +/// `finalize_pending` downgrade rationale in `unshield`); never fund +/// loss. The trade is deliberate: UX for the dominant offline case over +/// strict conservatism in a rare race. +/// /// Everything else leaves the outcome unknown and the caller must fall /// through to the result wait instead of failing: `AlreadyExists` proves /// the tx IS in the mempool or on chain (a lost-ACK attempt was re-sent