diff --git a/packages/rs-platform-wallet-ffi/src/error.rs b/packages/rs-platform-wallet-ffi/src/error.rs index 8e7e410022..8c6412cb03 100644 --- a/packages/rs-platform-wallet-ffi/src/error.rs +++ b/packages/rs-platform-wallet-ffi/src/error.rs @@ -97,6 +97,33 @@ 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 + /// 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 + /// 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, + /// 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 503054c533..d2ec755db7 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; @@ -314,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 @@ -372,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 @@ -434,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 @@ -467,6 +483,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 +507,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 +609,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}"), @@ -1107,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/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 38bf8ed179..598b1b4283 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -186,6 +186,43 @@ 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, + }, + + /// 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 f15a5bf1c3..cad0c71c86 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?; @@ -757,21 +769,110 @@ 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). 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 returned no verdict; the \ + transition may have been admitted — falling through to the result wait" + ); + } + } + + // Wait for proven execution. Classify the failure: + // - `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). 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 - .map_err(|e| PlatformWalletError::ShieldedBroadcastFailed(e.to_string()))?; + { + Ok(result) => result, + Err(dash_sdk::Error::StateTransitionBroadcastError(e)) if e.cause.is_some() => { + 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 @@ -846,6 +947,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 +963,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) // ------------------------------------------------------------------------- @@ -1081,6 +1242,192 @@ 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 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). +/// +/// 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> { + 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) + .await + .map(|_| ()) + .map_err(|wait_err| classify_spend_wait_failure(operation, &wait_err)) +} + +/// 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: +/// +/// - 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 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 +/// 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, + } +} + +/// Classify a `wait_for_response` failure for an already-broadcast +/// shielded spend (see [`broadcast_shielded_spend`]). +/// +/// 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 +/// 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 { + 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(), + } + } +} + /// Helper to clone selection results out from under the store lock. trait SelectionResultOwned { fn into_owned(self) -> (Vec, u64, u64); @@ -1134,6 +1481,167 @@ 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", + .. + } + )); + } + + /// 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() { + 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!( + err, + PlatformWalletError::ShieldedBroadcastFailed(_) + )); + } + + 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_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 be treated as a failure + /// — the spend is in flight and the result wait will confirm it. + #[test] + fn broadcast_already_exists_is_in_flight() { + assert!(!broadcast_definitely_failed( + &dash_sdk::Error::AlreadyExists("state transition already in mempool".to_string()) + )); + } +} + #[cfg(test)] mod reserve_shield_fee_tests { use super::*; 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 7586526b08..b78a7d322e 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 8e9302b2e7..501a29a715 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 @@ -467,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, @@ -583,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, @@ -625,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, @@ -787,8 +829,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 942ef996dd..453cfd1f2d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swift @@ -21,6 +21,23 @@ public enum PlatformWalletResultCode: Int32, Sendable { case errorArithmeticOverflow = 13 case errorNoSelectableInputs = 14 case errorWalletAlreadyExists = 15 + /// 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 + /// 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 @@ -58,6 +75,12 @@ 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_ERROR_SHIELDED_SPEND_UNCONFIRMED: + self = .errorShieldedSpendUnconfirmed case PLATFORM_WALLET_FFI_RESULT_CODE_NOT_FOUND: self = .notFound case PLATFORM_WALLET_FFI_RESULT_CODE_ERROR_UNKNOWN: @@ -136,6 +159,22 @@ public enum PlatformWalletError: LocalizedError { case arithmeticOverflow(String) case noSelectableInputs(String) case walletAlreadyExists(String) + /// 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 + /// the derived id should special-case the + /// `.errorShieldedBroadcastUnconfirmed` result code (which carries + /// `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) @@ -149,7 +188,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), .shieldedSpendUnconfirmed(let m), .notFound(let m), .unknown(let m): return m } @@ -177,6 +217,9 @@ 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 .errorShieldedSpendUnconfirmed: self = .shieldedSpendUnconfirmed(detail) case .notFound: self = .notFound(detail) case .errorUnknown: self = .unknown(detail) } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/ViewModels/SendViewModel.swift index 86703a5517..45ecece47a 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 } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Services/IdentityRegistrationController.swift index 491cda0b72..513efed63b 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 ec1cff6564..e3fcba18dc 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 aec3cd8de9..14cdf71e66 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: @@ -1551,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 @@ -1944,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, diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/PendingRegistrationsList.swift index e1f60d47bc..3ccb6880e1 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 7b2a536b3f..e6f51019f3 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 1bbb2b1a85..c17cad58eb 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" + ) } }