From 89fbe619abfad781a294beb78889e5169ff1186e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 26 Apr 2026 15:26:08 +0800 Subject: [PATCH 1/5] feat(ios)!: derive+persist + sign-with-resolver Rust-owned pipelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the swift-sdk/CLAUDE.md "no mnemonic round-tripping" rule debt that was deferred from PR #3541. Two Swift call sites used to pull a BIP-39 mnemonic out of the iOS Keychain, hand it to a stateless Rust FFI, get derived bytes back, and write them to Keychain — exactly the Swift-side pipeline orchestration the rule was written to forbid. This PR pushes the orchestration loops down into Rust and replaces the cross-FFI mnemonic argument with a pair of Swift-owned callback handles (mnemonic resolver + identity-key persister) the Rust loop drives directly. Architecture: - New `MnemonicResolverHandle` + `IdentityKeyPersisterHandle` vtable + create/destroy FFIs in `derive_and_persist_callbacks.rs`. Mirrors the existing `SignerVTable` pattern from rs-sdk-ffi but synchronous (Keychain reads/writes are sub-millisecond, no oneshot/CompletionSlot needed). Resolver uses a Rust-owned `Zeroizing<[u8; 1024]>` buffer the Swift impl writes the mnemonic into. - `dash_sdk_derive_and_persist_identity_keys`: single Rust FFI that owns the per-key derivation loop, computes per-key DPP metadata (key_type / purpose / security_level) including the MASTER-vs-HIGH policy, persists each key via the persister callback, and returns ONLY pubkeys + paths. The 32-byte ECDSA scalars never flow back across the FFI. - `dash_sdk_sign_with_mnemonic_resolver_and_path`: sibling of the lower-level `dash_sdk_sign_with_mnemonic_and_path` that takes the resolver handle instead of a raw mnemonic C-string. Same zeroization story (mnemonic → seed → derived secret all live in `Zeroizing` buffers; only the signature crosses back). Swift side: - New `MnemonicResolver` and `IdentityKeyPersister` Swift wrapper classes (`FFI/MnemonicResolverAndPersister.swift`) using the same `Unmanaged.passRetained` ctx pattern `KeychainSigner` uses. The persister captures per-key metadata as a side effect so callers can build `IdentityPubkey` rows from what Rust decided rather than recreating the policy. - `prePersistIdentityKeysForRegistration` rewritten: - No longer pulls mnemonic into a Swift `String`. - No longer holds the per-key MASTER-vs-HIGH discriminant block that previously lived in Swift. - Returns `[IdentityPubkey]` directly (was `[IdentityRegistrationKeyPreview]` + caller had to `enumerated().map { ... securityLevel: i == 0 ? .master : .high ... }`). - `walletIdHex` parameter dropped — Rust ships raw 32-byte walletId to the persister, Swift hex-encodes inside the impl. - `KeychainSigner.signPlatformAddressOnDemand` migrated: - Holds a stored `MnemonicResolver` instance. - Calls `dash_sdk_sign_with_mnemonic_resolver_and_path` instead of `dash_sdk_sign_with_mnemonic_and_path`. - The mnemonic-fetching `try WalletStorage().retrieveMnemonic` +`mnemonic.withCString` block is gone. - `CreateIdentityView` simplified: the policy-recreation `enumerated().map` is gone; the call site just receives the ready-to-use `IdentityPubkey` rows. Lower-level FFIs preserved: - `dash_sdk_derive_identity_keys_from_mnemonic` and `dash_sdk_sign_with_mnemonic_and_path` are untouched. Per the spawn_task brief, they stay as primitives in case other consumers ever wire them up; removal is for a later cleanup PR once we're sure no one else uses them. Wire-shape note: The persister callback returns `u8` (0/1) rather than `bool` because Swift's `@convention(c)` typealiases require types representable in Objective-C. Neither Swift's `Bool` nor `DarwinBoolean` qualifies in that context; `UInt8` does and has the same single-byte ABI footprint as Rust `bool`. Same fix on the args pointer: `UnsafeRawPointer?` rather than `UnsafePointer?`, with `assumingMemoryBound` in the trampoline. Verification: - cargo clippy -p platform-wallet-ffi -p platform-wallet -p rs-sdk-ffi --all-targets -- -D warnings: clean - cargo test -p platform-wallet --lib: 110/110 pass - cargo test -p platform-wallet-ffi --lib: 58/58 pass (incl. 7 new unit tests covering both FFI entry points end-to-end) - cargo machete: clean across all three crates - ./build_ios.sh --target sim: SwiftExampleApp builds and links cleanly with the new FFI symbols Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/derive_and_persist_callbacks.rs | 468 +++++++++++ .../src/identity_derive_and_persist.rs | 727 ++++++++++++++++++ .../src/identity_keys_from_mnemonic.rs | 6 +- packages/rs-platform-wallet-ffi/src/lib.rs | 6 + .../src/sign_with_mnemonic_resolver.rs | 373 +++++++++ .../SwiftDashSDK/FFI/KeychainSigner.swift | 91 ++- .../FFI/MnemonicResolverAndPersister.swift | 288 +++++++ .../ManagedPlatformWallet.swift | 177 ++--- .../PlatformWallet/PlatformWalletFFI.swift | 189 +++++ .../Views/CreateIdentityView.swift | 38 +- 10 files changed, 2187 insertions(+), 176 deletions(-) create mode 100644 packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs create mode 100644 packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs create mode 100644 packages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs create mode 100644 packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift diff --git a/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs b/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs new file mode 100644 index 00000000000..b7b98d9da91 --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs @@ -0,0 +1,468 @@ +//! C-ABI callback handles backing the +//! [`crate::dash_sdk_derive_and_persist_identity_keys`] entry point. +//! +//! The architectural intent is captured in `swift-sdk/CLAUDE.md`'s +//! "no mnemonic round-tripping" rule: derivation pipelines must live +//! on the Rust side, with Swift exposing only the two operations +//! Rust cannot perform from its side — reading the mnemonic from +//! the iOS Keychain and writing the derived key bytes back. This +//! module exposes those two operations as paired vtable + handle +//! types so a Rust-owned derivation loop can call into Swift for +//! each one. +//! +//! # Why two separate handles +//! +//! Conceptually they could ride on a single vtable, but the +//! lifetimes are different: +//! +//! - The mnemonic resolver is single-use per call (one read of one +//! wallet's mnemonic per `dash_sdk_derive_and_persist_identity_keys` +//! invocation). +//! - The persister is multi-use per call (`key_count` writes per +//! invocation). +//! +//! Splitting them also makes future extensions cleaner: a hardware- +//! key-store impl might persist via a Secure Enclave wrapper that has +//! nothing in common with how the mnemonic is fetched. +//! +//! # Synchronous on purpose +//! +//! Unlike the +//! [`SignerVTable`](rs_sdk_ffi::SignerVTable) infrastructure (which +//! uses a `oneshot` channel + `CompletionSlot` because biometric +//! prompts can take seconds), Keychain reads and writes are +//! sub-millisecond, so both callbacks here are fully synchronous — +//! no completion-callback ceremony, no `tokio::time::timeout`. +//! The Rust side blocks the calling thread for the duration of one +//! Keychain hop. Callers that already invoke the derivation FFI on +//! a background queue (the iOS pattern) get exactly what they want. +//! +//! # Lifetime / cleanup +//! +//! The Swift side calls `dash_sdk_mnemonic_resolver_create` / +//! `dash_sdk_identity_key_persister_create`, which return opaque +//! `*mut MnemonicResolverHandle` / `*mut IdentityKeyPersisterHandle` +//! pointers. Pass them to +//! `dash_sdk_derive_and_persist_identity_keys`. Then call the +//! matching `_destroy` function — that fires the supplied destructor +//! callback (typically the `Unmanaged.fromOpaque(...).release()` +//! pattern Swift uses for its `KeychainSigner` ctx) so the Swift +//! object can drop. + +use std::ffi::c_void; +use std::os::raw::c_char; + +// --------------------------------------------------------------------------- +// Mnemonic resolver — Rust → Swift "fetch BIP-39 mnemonic for wallet_id" +// --------------------------------------------------------------------------- + +/// Maximum mnemonic length, in bytes (excluding the trailing NUL), +/// the resolver buffer can hold. +/// +/// 1024 bytes covers every BIP-39 wordlist with margin to spare: +/// the longest 24-word mnemonics in any supported language come in +/// well under that — Korean (the longest of the supported lists) +/// tops out near 700 bytes for a 24-word phrase. Picking a power- +/// of-two cap keeps the inline Rust stack buffer (used in +/// [`crate::dash_sdk_derive_and_persist_identity_keys`]) cheap to +/// zero-on-drop. +pub const MNEMONIC_RESOLVER_BUFFER_CAPACITY: usize = 1024; + +/// Resolver result codes returned by [`MnemonicResolveCallback`]. +/// +/// Mirrors the success/failure shape of `PlatformWalletFFIResult` +/// at a finer-grained level so the Rust side can distinguish +/// "Swift hit the buffer cap" from "Swift had no mnemonic stored +/// for this wallet". +pub mod mnemonic_resolver_result { + /// Mnemonic copied into the buffer; `out_len` was set. + pub const SUCCESS: i32 = 0; + /// The Swift side has no mnemonic stored for this `wallet_id`. + /// Surfaced to the Rust caller as + /// `PlatformWalletFFIResult::ErrorWalletOperation` with a + /// "mnemonic missing" detail. + pub const NOT_FOUND: i32 = 1; + /// Mnemonic exceeded [`MNEMONIC_RESOLVER_BUFFER_CAPACITY`]. + /// Should not happen in practice — the buffer is sized for + /// every BIP-39 wordlist's 24-word phrase plus margin. + pub const BUFFER_TOO_SMALL: i32 = 2; + /// Anything else (Keychain access denied, decode error, etc.). + /// Surfaced as `ErrorWalletOperation` with a generic detail + /// the Swift side hopefully logged before returning. + pub const OTHER: i32 = 3; +} + +/// Function pointer type for the mnemonic-resolve callback. +/// +/// # Wire shape +/// +/// - `ctx`: opaque Swift-side context (typically the result of +/// `Unmanaged.passRetained(swiftSelf).toOpaque()`). Passed back +/// verbatim by Rust on every invocation. +/// - `wallet_id_bytes`: pointer to a 32-byte wallet id. Valid for +/// the duration of the call only. +/// - `out_mnemonic_utf8`: writable buffer for the NUL-terminated +/// UTF-8 mnemonic. Capacity is +/// [`MNEMONIC_RESOLVER_BUFFER_CAPACITY`] bytes (room for the +/// trailing NUL). +/// - `out_capacity`: equal to [`MNEMONIC_RESOLVER_BUFFER_CAPACITY`]. +/// Surfaced explicitly so the implementation can sanity-check +/// without assuming the constant. +/// - `out_len`: receives the byte count written to +/// `out_mnemonic_utf8`, EXCLUDING the trailing NUL. Must be set +/// on success. +/// +/// # Safety +/// +/// - `out_mnemonic_utf8` must be valid for `out_capacity` writable +/// bytes for the duration of this call. +/// - `wallet_id_bytes` must be valid for 32 readable bytes. +/// - `out_len` must be valid for one `usize` write. +/// - The implementation MUST return one of the +/// [`mnemonic_resolver_result`] codes; any other value is treated +/// as `OTHER`. +pub type MnemonicResolveCallback = unsafe extern "C" fn( + ctx: *const c_void, + wallet_id_bytes: *const u8, + out_mnemonic_utf8: *mut c_char, + out_capacity: usize, + out_len: *mut usize, +) -> i32; + +/// C-compatible vtable for a mnemonic resolver. +#[repr(C)] +pub struct MnemonicResolverVTable { + /// Synchronous "fetch mnemonic for `wallet_id`". + pub resolve: MnemonicResolveCallback, + /// Destructor for the `ctx` pointer. Invoked exactly once when + /// the matching `dash_sdk_mnemonic_resolver_destroy` is called. + pub destroy: unsafe extern "C" fn(ctx: *mut c_void), +} + +/// Opaque Rust-side handle to a Swift-owned mnemonic resolver. +/// +/// Constructed by [`dash_sdk_mnemonic_resolver_create`], destroyed +/// by [`dash_sdk_mnemonic_resolver_destroy`]. Pass the pointer +/// into [`crate::dash_sdk_derive_and_persist_identity_keys`]. +#[repr(C)] +pub struct MnemonicResolverHandle { + pub(crate) ctx: *mut c_void, + pub(crate) vtable: *mut MnemonicResolverVTable, +} + +// SAFETY: Swift side promises both pointers are thread-stable for +// the lifetime of this handle. The handle is owned by Swift +// across the FFI; Rust just dereferences it under the same lock +// shape the existing signer FFI uses. +unsafe impl Send for MnemonicResolverHandle {} +unsafe impl Sync for MnemonicResolverHandle {} + +/// Build a new resolver handle that wraps a Swift-owned `ctx` +/// pointer plus a pair of function pointers. +/// +/// # Safety +/// - `resolve_callback` must conform to [`MnemonicResolveCallback`]'s +/// contract. +/// - `destroy_callback` must safely free `ctx` exactly once. +/// - `ctx` may be null (the destructor will be invoked with null +/// on destroy regardless). +#[no_mangle] +pub unsafe extern "C" fn dash_sdk_mnemonic_resolver_create( + ctx: *mut c_void, + resolve_callback: MnemonicResolveCallback, + destroy_callback: unsafe extern "C" fn(ctx: *mut c_void), +) -> *mut MnemonicResolverHandle { + // Allocate the vtable on the heap so it has a stable address + // for the lifetime of the handle — same pattern + // `dash_sdk_signer_create_with_ctx` follows. + let vtable = Box::into_raw(Box::new(MnemonicResolverVTable { + resolve: resolve_callback, + destroy: destroy_callback, + })); + Box::into_raw(Box::new(MnemonicResolverHandle { ctx, vtable })) +} + +/// Destroy a previously-created resolver handle. +/// +/// Calls the supplied destructor exactly once with the original +/// `ctx`, then frees the heap-allocated vtable and the handle box. +/// Safe to call with a null pointer (no-op). +/// +/// # Safety +/// `handle` must have been produced by +/// [`dash_sdk_mnemonic_resolver_create`] and must not have been +/// destroyed already. +#[no_mangle] +pub unsafe extern "C" fn dash_sdk_mnemonic_resolver_destroy(handle: *mut MnemonicResolverHandle) { + if handle.is_null() { + return; + } + let owned = unsafe { Box::from_raw(handle) }; + unsafe { + if !owned.vtable.is_null() { + ((*owned.vtable).destroy)(owned.ctx); + // Reclaim the vtable box. + let _ = Box::from_raw(owned.vtable); + } + } +} + +// --------------------------------------------------------------------------- +// Identity-key persister — Rust → Swift "save this derived key to Keychain" +// --------------------------------------------------------------------------- + +/// `#[repr(C)]` struct passed to [`PersistKeyCallback`]. Pointer- +/// based to keep the call-site stable if we ever add per-key +/// metadata fields without breaking the ABI. +/// +/// # Field invariants +/// +/// - `wallet_id_bytes` always points at 32 readable bytes. +/// - `derivation_path_cstr` is a NUL-terminated UTF-8 string of +/// the canonical DIP-9 form +/// (`"m/9'/coin'/5'/0'/0'/identity_index'/key_index'"`). +/// - `public_key_bytes` / `public_key_len` carry the raw +/// compressed secp256k1 pubkey (currently always 33 bytes; +/// the explicit length is defensive against future key types). +/// - `public_key_hash_bytes` always points at 20 readable bytes +/// (the HASH160 of the compressed pubkey, computed once on the +/// Rust side via [`crate::utils::platform_wallet_hash160`] so +/// Swift doesn't have to recompute it for the Keychain +/// metadata). +/// - `private_key_bytes` always points at 32 readable bytes +/// (the raw ECDSA scalar). Treat as sensitive — copy into the +/// Keychain immediately and let the local reference drop. +/// - `key_type` / `purpose` / `security_level` are the DPP +/// discriminant bytes (see +/// `dpp::identity::{KeyType, Purpose, SecurityLevel}`). For the +/// identity-registration flow this is always +/// `(ECDSA_SECP256K1=0, AUTHENTICATION=0, MASTER=0 | HIGH=2)` — +/// the MASTER-vs-HIGH choice belongs in Rust where DPP's enum +/// discriminants live, not Swift. +#[repr(C)] +pub struct PersistKeyArgs { + pub wallet_id_bytes: *const u8, + pub identity_index: u32, + pub key_id: u32, + pub key_index: u32, + pub derivation_path_cstr: *const c_char, + pub public_key_bytes: *const u8, + pub public_key_len: usize, + pub public_key_hash_bytes: *const u8, + pub private_key_bytes: *const u8, + pub key_type: u8, + pub purpose: u8, + pub security_level: u8, +} + +/// Function pointer type for the per-key persist callback. +/// +/// Returns a non-zero `u8` on a successful persist, `0` on +/// failure. On failure the Rust derivation loop aborts with an +/// `ErrorWalletOperation` and the partial state is the caller's +/// responsibility (in practice the Swift side either succeeds or +/// throws, not both). +/// +/// `u8` (rather than `bool`) keeps the wire shape representable +/// in Swift's `@convention(c)` typealiases, which can only carry +/// types representable in Objective-C — Swift's `Bool` and +/// `DarwinBoolean` are not, but `UInt8` is. Same ABI footprint. +/// +/// # Safety +/// - `args` must point at a valid [`PersistKeyArgs`] for the +/// duration of the call. All pointer fields inside `args` must +/// conform to their documented invariants. +/// - The implementation MUST NOT retain any of the pointer fields +/// past return — the underlying buffers (mnemonic, intermediate +/// xprivs, derived bytes) are zeroized on the Rust side as soon +/// as this function returns. +pub type PersistKeyCallback = + unsafe extern "C" fn(ctx: *const c_void, args: *const PersistKeyArgs) -> u8; + +/// Persist-callback success / failure tags. Mirrors a boolean +/// but in `u8` form so it crosses Swift's `@convention(c)` cleanly. +pub const PERSIST_KEY_SUCCESS: u8 = 1; +pub const PERSIST_KEY_FAILURE: u8 = 0; + +/// C-compatible vtable for an identity-key persister. +#[repr(C)] +pub struct IdentityKeyPersisterVTable { + pub persist_key: PersistKeyCallback, + pub destroy: unsafe extern "C" fn(ctx: *mut c_void), +} + +/// Opaque Rust-side handle to a Swift-owned identity-key persister. +#[repr(C)] +pub struct IdentityKeyPersisterHandle { + pub(crate) ctx: *mut c_void, + pub(crate) vtable: *mut IdentityKeyPersisterVTable, +} + +unsafe impl Send for IdentityKeyPersisterHandle {} +unsafe impl Sync for IdentityKeyPersisterHandle {} + +/// Build a new identity-key persister handle. +/// +/// # Safety +/// - `persist_callback` must conform to [`PersistKeyCallback`]'s +/// contract. +/// - `destroy_callback` must safely free `ctx` exactly once. +#[no_mangle] +pub unsafe extern "C" fn dash_sdk_identity_key_persister_create( + ctx: *mut c_void, + persist_callback: PersistKeyCallback, + destroy_callback: unsafe extern "C" fn(ctx: *mut c_void), +) -> *mut IdentityKeyPersisterHandle { + let vtable = Box::into_raw(Box::new(IdentityKeyPersisterVTable { + persist_key: persist_callback, + destroy: destroy_callback, + })); + Box::into_raw(Box::new(IdentityKeyPersisterHandle { ctx, vtable })) +} + +/// Destroy a previously-created persister handle. Safe on null. +/// +/// # Safety +/// `handle` must have been produced by +/// [`dash_sdk_identity_key_persister_create`] and must not have +/// been destroyed already. +#[no_mangle] +pub unsafe extern "C" fn dash_sdk_identity_key_persister_destroy( + handle: *mut IdentityKeyPersisterHandle, +) { + if handle.is_null() { + return; + } + let owned = unsafe { Box::from_raw(handle) }; + unsafe { + if !owned.vtable.is_null() { + ((*owned.vtable).destroy)(owned.ctx); + let _ = Box::from_raw(owned.vtable); + } + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + + static RESOLVER_CALLED: AtomicBool = AtomicBool::new(false); + static RESOLVER_DESTROYED: AtomicBool = AtomicBool::new(false); + static PERSISTER_CALLS: AtomicUsize = AtomicUsize::new(0); + static PERSISTER_DESTROYED: AtomicBool = AtomicBool::new(false); + + unsafe extern "C" fn fake_resolve( + _ctx: *const c_void, + _wallet_id_bytes: *const u8, + out_buf: *mut c_char, + out_capacity: usize, + out_len: *mut usize, + ) -> i32 { + RESOLVER_CALLED.store(true, Ordering::SeqCst); + let phrase = b"abandon abandon abandon abandon abandon abandon abandon abandon \ + abandon abandon abandon about"; + if phrase.len() + 1 > out_capacity { + return mnemonic_resolver_result::BUFFER_TOO_SMALL; + } + unsafe { + std::ptr::copy_nonoverlapping(phrase.as_ptr() as *const c_char, out_buf, phrase.len()); + *out_buf.add(phrase.len()) = 0; + *out_len = phrase.len(); + } + mnemonic_resolver_result::SUCCESS + } + + unsafe extern "C" fn fake_resolver_destroy(_ctx: *mut c_void) { + RESOLVER_DESTROYED.store(true, Ordering::SeqCst); + } + + unsafe extern "C" fn fake_persist(_ctx: *const c_void, _args: *const PersistKeyArgs) -> u8 { + PERSISTER_CALLS.fetch_add(1, Ordering::SeqCst); + PERSIST_KEY_SUCCESS + } + + unsafe extern "C" fn fake_persister_destroy(_ctx: *mut c_void) { + PERSISTER_DESTROYED.store(true, Ordering::SeqCst); + } + + #[test] + fn resolver_create_destroy_roundtrip() { + unsafe { + let h = dash_sdk_mnemonic_resolver_create( + std::ptr::null_mut(), + fake_resolve, + fake_resolver_destroy, + ); + assert!(!h.is_null()); + assert!(!(*h).vtable.is_null()); + + // Drive the resolver once to confirm the wired-up callback fires. + let mut buf = [0u8; MNEMONIC_RESOLVER_BUFFER_CAPACITY]; + let mut len: usize = 0; + let rc = ((*(*h).vtable).resolve)( + (*h).ctx, + [0u8; 32].as_ptr(), + buf.as_mut_ptr() as *mut c_char, + buf.len(), + &mut len, + ); + assert_eq!(rc, mnemonic_resolver_result::SUCCESS); + assert!(len > 0); + + dash_sdk_mnemonic_resolver_destroy(h); + } + assert!(RESOLVER_CALLED.load(Ordering::SeqCst)); + assert!(RESOLVER_DESTROYED.load(Ordering::SeqCst)); + } + + #[test] + fn persister_create_destroy_roundtrip() { + unsafe { + let h = dash_sdk_identity_key_persister_create( + std::ptr::null_mut(), + fake_persist, + fake_persister_destroy, + ); + assert!(!h.is_null()); + + let pubkey = [0u8; 33]; + let pubhash = [0u8; 20]; + let privkey = [0u8; 32]; + let path = b"m/9'/1'/5'/0'/0'/0'/0'\0"; + let wallet_id = [0u8; 32]; + let args = PersistKeyArgs { + wallet_id_bytes: wallet_id.as_ptr(), + identity_index: 0, + key_id: 0, + key_index: 0, + derivation_path_cstr: path.as_ptr() as *const c_char, + public_key_bytes: pubkey.as_ptr(), + public_key_len: pubkey.len(), + public_key_hash_bytes: pubhash.as_ptr(), + private_key_bytes: privkey.as_ptr(), + key_type: 0, + purpose: 0, + security_level: 0, + }; + let ok = ((*(*h).vtable).persist_key)((*h).ctx, &args); + assert_eq!(ok, PERSIST_KEY_SUCCESS); + dash_sdk_identity_key_persister_destroy(h); + } + assert_eq!(PERSISTER_CALLS.load(Ordering::SeqCst), 1); + assert!(PERSISTER_DESTROYED.load(Ordering::SeqCst)); + } + + #[test] + fn destroy_handles_null() { + unsafe { + // Should be no-ops. + dash_sdk_mnemonic_resolver_destroy(std::ptr::null_mut()); + dash_sdk_identity_key_persister_destroy(std::ptr::null_mut()); + } + } +} diff --git a/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs b/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs new file mode 100644 index 00000000000..cbe4631d7c4 --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs @@ -0,0 +1,727 @@ +//! Single-FFI identity-key derivation + persistence pipeline. +//! +//! Companion to [`crate::derive_and_persist_callbacks`] (which +//! defines the resolver / persister vtables) and the lower-level +//! [`crate::dash_sdk_derive_identity_keys_from_mnemonic`] (which +//! returns the derived keys to the caller and lets the caller +//! orchestrate persistence). +//! +//! # Why this entry point exists +//! +//! `swift-sdk/CLAUDE.md` forbids the Swift SDK from orchestrating +//! the BIP-39 → seed → master xpriv → derived xpriv → 32-byte +//! private key → Keychain pipeline. The lower-level FFI is enough +//! for Swift to *call* per-key derivation, but Swift then has to +//! pull the mnemonic out of Keychain itself, hand it across the +//! FFI as a C-string, and walk the returned rows writing each to +//! Keychain — i.e. the rule violation the rule was written for. +//! +//! This entry point pushes the loop down: Swift hands the Rust side +//! two callback handles (one resolver, one persister) and the Rust +//! side runs the entire pipeline, calling back into Swift only for +//! the iOS-only Keychain primitives. The Swift caller's +//! responsibility shrinks to: +//! +//! 1. Implement the resolver: given a `wallet_id`, copy the +//! BIP-39 mnemonic into a Rust-owned buffer. +//! 2. Implement the persister: given a `(walletId, identityIndex, +//! keyId, path, pubkey, pubkeyHash, privKey, keyType, purpose, +//! securityLevel)` tuple, write a Keychain row. +//! +//! The mnemonic still crosses the FFI as a C-string (Keychain is +//! iOS-only, so Rust can't read it directly) but it now flows IN +//! to a Rust-owned `Zeroizing` buffer that is scrubbed at function +//! exit. The 32-byte ECDSA scalars never flow OUT — they stay on +//! the Rust stack until the persister callback writes them, and +//! are zeroized as soon as the loop iteration completes. +//! +//! # Policy lives in Rust +//! +//! Per-key metadata (key_type / purpose / security_level) is +//! computed on the Rust side using the DPP discriminants directly, +//! removing the policy-encoded-in-Swift block that the lower- +//! level FFI required the Swift caller to maintain. For the +//! identity-registration flow: +//! +//! | key_id | key_type | purpose | security_level | +//! |--------|-----------------|----------------|----------------| +//! | 0 | ECDSA_SECP256K1 | AUTHENTICATION | MASTER | +//! | > 0 | ECDSA_SECP256K1 | AUTHENTICATION | HIGH | +//! +//! If DPP renumbers any of those enum discriminants, this file is +//! the single place that needs updating — the Swift caller just +//! writes whatever bytes it's told. +//! +//! # Output shape +//! +//! Returns a heap-allocated [`IdentityRegistrationKeyDerivationsFFI`] +//! containing pubkeys + paths only — `private_key_bytes` is left +//! at the zero-initialised default (`[0u8; 32]`) and +//! `private_key_wif` is null. The `_free` function on the existing +//! struct handles the zero/null case correctly. The returned rows +//! exist so the Swift caller can build its `IdentityPubkey` rows +//! for the upcoming `registerIdentityFromAddresses(...signer:)` +//! call without performing a second derivation pass. + +use std::ffi::{c_void, CString}; +use std::os::raw::c_char; +use std::ptr; + +use dashcore::hashes::Hash; +use dashcore::secp256k1::Secp256k1; +use key_wallet::bip32::{ExtendedPrivKey, ExtendedPubKey}; +use rs_sdk_ffi::DashSDKNetwork; +use zeroize::{Zeroize, Zeroizing}; + +use crate::derive_and_persist_callbacks::{ + mnemonic_resolver_result, IdentityKeyPersisterHandle, MnemonicResolverHandle, PersistKeyArgs, + MNEMONIC_RESOLVER_BUFFER_CAPACITY, PERSIST_KEY_SUCCESS, +}; +use crate::error::*; +use crate::identity_key_preview::IdentityKeyPreviewFFI; +use crate::identity_keys_from_mnemonic::{ + identity_auth_derivation_path, map_network, parse_mnemonic_any_language, +}; +use crate::identity_registration_with_signer::IdentityRegistrationKeyDerivationsFFI; + +/// DPP `KeyType::ECDSA_SECP256K1` discriminant byte. +const KEY_TYPE_ECDSA_SECP256K1: u8 = 0; +/// DPP `Purpose::AUTHENTICATION` discriminant byte. +const PURPOSE_AUTHENTICATION: u8 = 0; +/// DPP `SecurityLevel::MASTER` discriminant byte. +const SECURITY_LEVEL_MASTER: u8 = 0; +/// DPP `SecurityLevel::HIGH` discriminant byte. +const SECURITY_LEVEL_HIGH: u8 = 2; + +/// Derive `key_count` identity-registration keypairs at DIP-9 +/// paths and persist each one's private key to the Swift-owned +/// Keychain via the supplied callback handles. +/// +/// On success, `out_pubkeys` is populated with the derived pubkeys +/// (and their paths) for the caller to build `IdentityPubkey` rows +/// for the subsequent registration call. The 32-byte ECDSA private +/// scalars are NOT included in the output — they were already +/// handed to the persister callback per iteration. +/// +/// # Parameters +/// +/// - `network`: selects the DIP-9 coin slot AND the WIF version +/// byte (the persister callback receives a `key_type` byte and +/// can derive WIF on the Swift side if needed). +/// - `wallet_id_bytes`: pointer to a 32-byte wallet id. Forwarded +/// to both callbacks. Required even though the resolver needs +/// it most — the persister callback receives it too so the +/// Swift impl can stamp `walletId` into Keychain metadata +/// without remembering which call this row came from. +/// - `identity_index`: BIP-9 identity index slot. +/// - `key_count`: number of consecutive `key_index` slots to +/// derive, starting at 0. +/// - `mnemonic_resolver_handle`: opaque handle from +/// [`crate::dash_sdk_mnemonic_resolver_create`]. +/// - `persister_handle`: opaque handle from +/// [`crate::dash_sdk_identity_key_persister_create`]. +/// - `out_pubkeys`: populated on success with a heap-allocated +/// array of pubkey-only rows. Release with +/// [`crate::dash_sdk_derive_identity_keys_from_mnemonic_free`] +/// — same memory layout as the lower-level FFI's output, so the +/// existing free path handles it correctly. +/// - `out_error`: populated on failure with the usual +/// [`PlatformWalletFFIError`] detail. +/// +/// On error `*out_pubkeys` is left at the empty zero state. Any +/// keys persisted by partial success before the failing iteration +/// are NOT rolled back — the persister callback is single-shot +/// per key. The Swift caller is expected to surface the error and +/// not proceed to registration; orphan Keychain rows from a +/// partially-failed call are harmless (they're just unused +/// derived material). +/// +/// # Safety +/// +/// - `wallet_id_bytes` must be valid for 32 readable bytes. +/// - Both handle pointers must come from their respective `create` +/// functions and remain valid for the duration of the call. +/// - `out_pubkeys` must be a valid, writable +/// [`IdentityRegistrationKeyDerivationsFFI`] pointer. +/// - `out_error` may be null. +#[no_mangle] +#[allow(clippy::too_many_arguments)] +pub unsafe extern "C" fn dash_sdk_derive_and_persist_identity_keys( + network: DashSDKNetwork, + wallet_id_bytes: *const u8, + identity_index: u32, + key_count: u32, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + persister_handle: *mut IdentityKeyPersisterHandle, + out_pubkeys: *mut IdentityRegistrationKeyDerivationsFFI, + out_error: *mut PlatformWalletFFIError, +) -> PlatformWalletFFIResult { + if out_pubkeys.is_null() { + write_err( + out_error, + PlatformWalletFFIResult::ErrorNullPointer, + "out_pubkeys is null", + ); + return PlatformWalletFFIResult::ErrorNullPointer; + } + // Pre-zero so a failed call leaves the caller staring at a known + // empty struct, never uninitialized memory. + *out_pubkeys = IdentityRegistrationKeyDerivationsFFI { + items: ptr::null_mut(), + count: 0, + }; + + if wallet_id_bytes.is_null() || mnemonic_resolver_handle.is_null() || persister_handle.is_null() + { + write_err( + out_error, + PlatformWalletFFIResult::ErrorNullPointer, + "wallet_id_bytes, mnemonic_resolver_handle and persister_handle are required", + ); + return PlatformWalletFFIResult::ErrorNullPointer; + } + + if key_count == 0 { + return PlatformWalletFFIResult::Success; + } + + // ---- Resolve mnemonic ---------------------------------------------------- + // Stack-resident, zeroized-on-drop buffer the resolver writes into. + let mut mnemonic_buf: Zeroizing<[u8; MNEMONIC_RESOLVER_BUFFER_CAPACITY]> = + Zeroizing::new([0u8; MNEMONIC_RESOLVER_BUFFER_CAPACITY]); + let mut mnemonic_len: usize = 0; + + let resolver = &*mnemonic_resolver_handle; + let resolver_vtable = &*resolver.vtable; + let rc = (resolver_vtable.resolve)( + resolver.ctx as *const c_void, + wallet_id_bytes, + mnemonic_buf.as_mut_ptr() as *mut c_char, + MNEMONIC_RESOLVER_BUFFER_CAPACITY, + &mut mnemonic_len, + ); + match rc { + x if x == mnemonic_resolver_result::SUCCESS => {} + x if x == mnemonic_resolver_result::NOT_FOUND => { + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + "mnemonic resolver: no mnemonic stored for the supplied wallet_id", + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + x if x == mnemonic_resolver_result::BUFFER_TOO_SMALL => { + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + "mnemonic resolver: mnemonic exceeded the FFI buffer capacity", + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + _ => { + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + "mnemonic resolver: failed (other / Keychain access error)", + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + } + if mnemonic_len == 0 || mnemonic_len > MNEMONIC_RESOLVER_BUFFER_CAPACITY { + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + "mnemonic resolver: returned invalid length", + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + + // Validate UTF-8 once over the prefix the resolver claimed to + // write. Done in-place so we never construct a `String` + // (Swift's String can't be zeroized; ours can). + let mnemonic_str = match std::str::from_utf8(&mnemonic_buf[..mnemonic_len]) { + Ok(s) => s, + Err(e) => { + write_err( + out_error, + PlatformWalletFFIResult::ErrorUtf8Conversion, + format!("mnemonic resolver returned non-UTF-8 bytes: {e}"), + ); + return PlatformWalletFFIResult::ErrorUtf8Conversion; + } + }; + let mnemonic = match parse_mnemonic_any_language(mnemonic_str) { + Ok(m) => m, + Err(e) => { + write_err( + out_error, + PlatformWalletFFIResult::ErrorInvalidParameter, + format!("mnemonic resolver returned an invalid BIP-39 phrase: {e}"), + ); + return PlatformWalletFFIResult::ErrorInvalidParameter; + } + }; + + // ---- Derive seed + master xpriv ------------------------------------------ + // Empty passphrase to mirror the rest of the SDK (no caller + // surface for a BIP-39 passphrase yet). + let seed: Zeroizing<[u8; 64]> = Zeroizing::new(mnemonic.to_seed("")); + // Mnemonic is no longer needed; explicit drop releases its + // (non-zeroized) `String` storage early. + drop(mnemonic); + + let kw_network = map_network(network); + let master = match ExtendedPrivKey::new_master(kw_network, seed.as_ref()) { + Ok(m) => m, + Err(e) => { + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + format!("ExtendedPrivKey::new_master failed: {e}"), + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + }; + let secp = Secp256k1::new(); + + // ---- Walk derivation paths, persist, build pubkey-only rows -------------- + let persister = &*persister_handle; + let persister_vtable = &*persister.vtable; + + let mut rows: Vec = Vec::with_capacity(key_count as usize); + + // Hand-roll cleanup: each successfully-pushed row owns a + // CString allocation (path) + a heap-allocated pubkey buffer + // that won't be freed by `Vec::drop`. + let cleanup = |rows: Vec| { + for row in rows { + if !row.derivation_path.is_null() { + let _ = CString::from_raw(row.derivation_path); + } + if !row.public_key.is_null() && row.public_key_len > 0 { + let _ = Vec::from_raw_parts(row.public_key, row.public_key_len, row.public_key_len); + } + // private_key_bytes is inline zeros; no WIF allocated. + } + }; + + for key_index in 0..key_count { + let path = match identity_auth_derivation_path(kw_network, identity_index, key_index) { + Ok(p) => p, + Err(detail) => { + cleanup(rows); + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + format!( + "derive_and_persist: path build failed at \ + (identity={identity_index}, key={key_index}): {detail}" + ), + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + }; + + let derived = match master.derive_priv(&secp, &path) { + Ok(d) => d, + Err(e) => { + cleanup(rows); + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + format!( + "derive_and_persist: derive_priv failed at \ + (identity={identity_index}, key={key_index}): {e}" + ), + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + }; + + // Materialize pubkey + hash160 once. + let extended_pub = ExtendedPubKey::from_priv(&secp, &derived); + let pub_bytes: [u8; 33] = extended_pub.public_key.serialize(); + let pub_hash: [u8; 20] = dashcore::hashes::hash160::Hash::hash(&pub_bytes).to_byte_array(); + + // Hold the secret scalar in a buffer that drops with + // `zeroize::Zeroize::zeroize` at scope end. + let mut priv_scalar: [u8; 32] = derived.private_key.secret_bytes(); + + let path_cstring = match CString::new(path.to_string()) { + Ok(s) => s, + Err(e) => { + priv_scalar.zeroize(); + cleanup(rows); + write_err( + out_error, + PlatformWalletFFIResult::ErrorUtf8Conversion, + format!("derivation path contained NUL byte: {e}"), + ); + return PlatformWalletFFIResult::ErrorUtf8Conversion; + } + }; + + // Per-key DPP metadata. MASTER on key_id 0, HIGH on the + // rest — the convention `register_identity` enforced + // before the loop moved out of the platform-wallet crate. + let security_level = if key_index == 0 { + SECURITY_LEVEL_MASTER + } else { + SECURITY_LEVEL_HIGH + }; + let args = PersistKeyArgs { + wallet_id_bytes, + identity_index, + key_id: key_index, + key_index, + derivation_path_cstr: path_cstring.as_ptr(), + public_key_bytes: pub_bytes.as_ptr(), + public_key_len: pub_bytes.len(), + public_key_hash_bytes: pub_hash.as_ptr(), + private_key_bytes: priv_scalar.as_ptr(), + key_type: KEY_TYPE_ECDSA_SECP256K1, + purpose: PURPOSE_AUTHENTICATION, + security_level, + }; + let persist_rc = + (persister_vtable.persist_key)(persister.ctx as *const c_void, &args as *const _); + + // Scrub the secret scalar before doing anything else + // (success or failure). The persister callback is + // contractually NOT allowed to retain the pointer past + // its return, so zeroizing here is safe. + priv_scalar.zeroize(); + + if persist_rc != PERSIST_KEY_SUCCESS { + // Path CString hasn't been transferred to the row yet; + // it falls out of scope here. + drop(path_cstring); + cleanup(rows); + write_err( + out_error, + PlatformWalletFFIResult::ErrorWalletOperation, + format!( + "persister callback returned false at \ + (identity={identity_index}, key={key_index})" + ), + ); + return PlatformWalletFFIResult::ErrorWalletOperation; + } + + // Detach the pubkey buffer for the FFI return. + let mut pub_box: Box<[u8]> = pub_bytes.to_vec().into_boxed_slice(); + let pub_ptr = pub_box.as_mut_ptr(); + let pub_len = pub_box.len(); + std::mem::forget(pub_box); + + rows.push(IdentityKeyPreviewFFI { + identity_index, + derivation_path: path_cstring.into_raw(), + public_key: pub_ptr, + public_key_len: pub_len, + // Intentionally null — the private key was already + // shipped to the persister callback. The free function + // tolerates null. + private_key_wif: ptr::null_mut(), + private_key_bytes: [0u8; 32], + }); + } + + // Hand the rows off as a heap allocation. Same memory layout + // as `dash_sdk_derive_identity_keys_from_mnemonic` produces — + // releasable with the same `_free` function. + let mut boxed_items = rows.into_boxed_slice(); + let items_ptr = boxed_items.as_mut_ptr(); + let items_count = boxed_items.len(); + std::mem::forget(boxed_items); + + *out_pubkeys = IdentityRegistrationKeyDerivationsFFI { + items: items_ptr, + count: items_count, + }; + + PlatformWalletFFIResult::Success +} + +/// Convenience for stamping an error into `out_error` if it's +/// non-null. Mirrors the inline pattern the rest of the FFI +/// surface uses; kept local since the rest of the file is full of +/// repetitive `if !out_error.is_null()` blocks otherwise. +unsafe fn write_err( + out_error: *mut PlatformWalletFFIError, + code: PlatformWalletFFIResult, + detail: impl Into, +) { + if !out_error.is_null() { + *out_error = PlatformWalletFFIError::new(code, detail.into()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::derive_and_persist_callbacks::{ + dash_sdk_identity_key_persister_create, dash_sdk_identity_key_persister_destroy, + dash_sdk_mnemonic_resolver_create, dash_sdk_mnemonic_resolver_destroy, + }; + use std::ffi::CStr; + use std::sync::Mutex; + + /// English BIP-39 test vector (all-zero entropy). Same fixture + /// the upstream `bip39` crate uses; reused here so the + /// derivation produces a known seed. + const ENGLISH_PHRASE: &str = + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + + /// Capture for what the persister callback received. Behind a + /// Mutex because the FFI callback is `extern "C"` and can't + /// hold a `&mut`. + static CAPTURED: Mutex> = Mutex::new(Vec::new()); + + #[derive(Debug, Clone)] + struct CapturedPersist { + identity_index: u32, + key_id: u32, + key_index: u32, + path: String, + public_key: Vec, + public_key_hash: Vec, + private_key: Vec, + key_type: u8, + purpose: u8, + security_level: u8, + wallet_id: [u8; 32], + } + + unsafe extern "C" fn capturing_resolve( + _ctx: *const c_void, + _wallet_id_bytes: *const u8, + out_buf: *mut c_char, + out_capacity: usize, + out_len: *mut usize, + ) -> i32 { + let phrase = ENGLISH_PHRASE.as_bytes(); + if phrase.len() + 1 > out_capacity { + return mnemonic_resolver_result::BUFFER_TOO_SMALL; + } + std::ptr::copy_nonoverlapping(phrase.as_ptr() as *const c_char, out_buf, phrase.len()); + *out_buf.add(phrase.len()) = 0; + *out_len = phrase.len(); + mnemonic_resolver_result::SUCCESS + } + + unsafe extern "C" fn noop_destroy(_ctx: *mut c_void) {} + + unsafe extern "C" fn capturing_persist(_ctx: *const c_void, args: *const PersistKeyArgs) -> u8 { + let a = &*args; + let path = CStr::from_ptr(a.derivation_path_cstr) + .to_string_lossy() + .into_owned(); + let public_key = std::slice::from_raw_parts(a.public_key_bytes, a.public_key_len).to_vec(); + let public_key_hash = std::slice::from_raw_parts(a.public_key_hash_bytes, 20).to_vec(); + let private_key = std::slice::from_raw_parts(a.private_key_bytes, 32).to_vec(); + let mut wallet_id = [0u8; 32]; + std::ptr::copy_nonoverlapping(a.wallet_id_bytes, wallet_id.as_mut_ptr(), 32); + CAPTURED.lock().unwrap().push(CapturedPersist { + identity_index: a.identity_index, + key_id: a.key_id, + key_index: a.key_index, + path, + public_key, + public_key_hash, + private_key, + key_type: a.key_type, + purpose: a.purpose, + security_level: a.security_level, + wallet_id, + }); + PERSIST_KEY_SUCCESS + } + + unsafe extern "C" fn failing_persist(_ctx: *const c_void, _args: *const PersistKeyArgs) -> u8 { + crate::derive_and_persist_callbacks::PERSIST_KEY_FAILURE + } + + fn make_handles( + persist: crate::derive_and_persist_callbacks::PersistKeyCallback, + ) -> (*mut MnemonicResolverHandle, *mut IdentityKeyPersisterHandle) { + unsafe { + ( + dash_sdk_mnemonic_resolver_create( + std::ptr::null_mut(), + capturing_resolve, + noop_destroy, + ), + dash_sdk_identity_key_persister_create(std::ptr::null_mut(), persist, noop_destroy), + ) + } + } + + #[test] + fn happy_path_persists_three_keys_and_returns_pubkeys() { + CAPTURED.lock().unwrap().clear(); + let (resolver, persister) = make_handles(capturing_persist); + let wallet_id = [42u8; 32]; + let mut out = IdentityRegistrationKeyDerivationsFFI { + items: std::ptr::null_mut(), + count: 0, + }; + let mut err = PlatformWalletFFIError::success(); + let rc = unsafe { + dash_sdk_derive_and_persist_identity_keys( + DashSDKNetwork::SDKTestnet, + wallet_id.as_ptr(), + 7, // identity_index + 3, // key_count + resolver, + persister, + &mut out, + &mut err, + ) + }; + assert_eq!(rc, PlatformWalletFFIResult::Success); + assert_eq!(out.count, 3); + + let captured = CAPTURED.lock().unwrap().clone(); + assert_eq!(captured.len(), 3); + + // key_id, key_index sequence + for (i, row) in captured.iter().enumerate() { + assert_eq!(row.identity_index, 7); + assert_eq!(row.key_id, i as u32); + assert_eq!(row.key_index, i as u32); + assert_eq!(row.key_type, KEY_TYPE_ECDSA_SECP256K1); + assert_eq!(row.purpose, PURPOSE_AUTHENTICATION); + assert_eq!( + row.security_level, + if i == 0 { + SECURITY_LEVEL_MASTER + } else { + SECURITY_LEVEL_HIGH + } + ); + assert_eq!(row.public_key.len(), 33); + assert_eq!(row.public_key_hash.len(), 20); + assert_eq!(row.private_key.len(), 32); + assert_eq!(row.wallet_id, wallet_id); + // hash160 sanity check: matches what the helper would compute. + let expected_hash = + dashcore::hashes::hash160::Hash::hash(&row.public_key).to_byte_array(); + assert_eq!(row.public_key_hash.as_slice(), expected_hash.as_slice()); + // Path shape: testnet coin = 1. + assert_eq!(row.path, format!("m/9'/1'/5'/0'/0'/7'/{}'", i)); + } + + unsafe { + crate::dash_sdk_derive_identity_keys_from_mnemonic_free(&mut out); + dash_sdk_mnemonic_resolver_destroy(resolver); + dash_sdk_identity_key_persister_destroy(persister); + } + } + + #[test] + fn returning_false_from_persister_aborts_with_wallet_op_error() { + CAPTURED.lock().unwrap().clear(); + let (resolver, persister) = make_handles(failing_persist); + let wallet_id = [1u8; 32]; + let mut out = IdentityRegistrationKeyDerivationsFFI { + items: std::ptr::null_mut(), + count: 0, + }; + let mut err = PlatformWalletFFIError::success(); + let rc = unsafe { + dash_sdk_derive_and_persist_identity_keys( + DashSDKNetwork::SDKTestnet, + wallet_id.as_ptr(), + 0, + 2, + resolver, + persister, + &mut out, + &mut err, + ) + }; + assert_eq!(rc, PlatformWalletFFIResult::ErrorWalletOperation); + assert_eq!(out.count, 0); + assert!(out.items.is_null()); + unsafe { + dash_sdk_mnemonic_resolver_destroy(resolver); + dash_sdk_identity_key_persister_destroy(persister); + } + } + + #[test] + fn rejects_null_inputs() { + let (resolver, persister) = make_handles(capturing_persist); + let wallet_id = [0u8; 32]; + let mut err = PlatformWalletFFIError::success(); + // Null out_pubkeys. + let rc = unsafe { + dash_sdk_derive_and_persist_identity_keys( + DashSDKNetwork::SDKTestnet, + wallet_id.as_ptr(), + 0, + 1, + resolver, + persister, + std::ptr::null_mut(), + &mut err, + ) + }; + assert_eq!(rc, PlatformWalletFFIResult::ErrorNullPointer); + + // Null wallet id. + let mut out = IdentityRegistrationKeyDerivationsFFI { + items: std::ptr::null_mut(), + count: 0, + }; + let rc = unsafe { + dash_sdk_derive_and_persist_identity_keys( + DashSDKNetwork::SDKTestnet, + std::ptr::null(), + 0, + 1, + resolver, + persister, + &mut out, + &mut err, + ) + }; + assert_eq!(rc, PlatformWalletFFIResult::ErrorNullPointer); + unsafe { + dash_sdk_mnemonic_resolver_destroy(resolver); + dash_sdk_identity_key_persister_destroy(persister); + } + } + + #[test] + fn key_count_zero_is_success_no_calls() { + CAPTURED.lock().unwrap().clear(); + let (resolver, persister) = make_handles(capturing_persist); + let wallet_id = [0u8; 32]; + let mut out = IdentityRegistrationKeyDerivationsFFI { + items: std::ptr::null_mut(), + count: 0, + }; + let mut err = PlatformWalletFFIError::success(); + let rc = unsafe { + dash_sdk_derive_and_persist_identity_keys( + DashSDKNetwork::SDKTestnet, + wallet_id.as_ptr(), + 0, + 0, + resolver, + persister, + &mut out, + &mut err, + ) + }; + assert_eq!(rc, PlatformWalletFFIResult::Success); + assert_eq!(out.count, 0); + assert_eq!(CAPTURED.lock().unwrap().len(), 0); + unsafe { + dash_sdk_mnemonic_resolver_destroy(resolver); + dash_sdk_identity_key_persister_destroy(persister); + } + } +} diff --git a/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs b/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs index f3b962468cb..019bc42d68f 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs @@ -60,7 +60,7 @@ use crate::identity_registration_with_signer::IdentityRegistrationKeyDerivations /// helper. BIP-39 wordlists are mutually exclusive within a single /// phrase, so the first language that yields a valid mnemonic is the /// right one. -fn parse_mnemonic_any_language(phrase: &str) -> Result { +pub(crate) fn parse_mnemonic_any_language(phrase: &str) -> Result { const LANGUAGES: [Language; 10] = [ Language::English, Language::Spanish, @@ -85,7 +85,7 @@ fn parse_mnemonic_any_language(phrase: &str) -> Result { /// /// `Local` collapses to `Regtest` to match the rest of the FFI surface /// (`dash_sdk_signer_create_from_private_key`, `dash_sdk_sign_with_mnemonic_and_path`). -fn map_network(network: DashSDKNetwork) -> key_wallet::Network { +pub(crate) fn map_network(network: DashSDKNetwork) -> key_wallet::Network { match network { DashSDKNetwork::SDKMainnet => key_wallet::Network::Mainnet, DashSDKNetwork::SDKTestnet => key_wallet::Network::Testnet, @@ -108,7 +108,7 @@ fn map_network(network: DashSDKNetwork) -> key_wallet::Network { /// Re-derived locally instead of borrowed from `platform-wallet` because /// `platform_wallet::identity_auth_derivation_path` is `pub(crate)` and /// this entry point sits outside that crate. -fn identity_auth_derivation_path( +pub(crate) fn identity_auth_derivation_path( network: key_wallet::Network, identity_index: u32, key_index: u32, diff --git a/packages/rs-platform-wallet-ffi/src/lib.rs b/packages/rs-platform-wallet-ffi/src/lib.rs index f8e24af37c8..011dd5b74eb 100644 --- a/packages/rs-platform-wallet-ffi/src/lib.rs +++ b/packages/rs-platform-wallet-ffi/src/lib.rs @@ -18,11 +18,13 @@ pub mod core_wallet_types; pub mod dashpay; pub mod dashpay_profile; pub mod derivation; +pub mod derive_and_persist_callbacks; pub mod dpns; pub mod error; pub mod established_contact; pub mod event_handler; pub mod handle; +pub mod identity_derive_and_persist; pub mod identity_discovery; pub mod identity_key_preview; pub mod identity_keys_from_mnemonic; @@ -43,6 +45,7 @@ pub mod platform_address_types; pub mod platform_addresses; pub mod platform_wallet_info; mod runtime; +pub mod sign_with_mnemonic_resolver; pub mod spv; pub mod types; pub mod utils; @@ -60,11 +63,13 @@ pub use core_wallet_types::*; pub use dashpay::*; pub use dashpay_profile::*; pub use derivation::*; +pub use derive_and_persist_callbacks::*; pub use dpns::*; pub use error::*; pub use established_contact::*; pub use event_handler::*; pub use handle::*; +pub use identity_derive_and_persist::*; pub use identity_discovery::*; pub use identity_key_preview::*; pub use identity_keys_from_mnemonic::*; @@ -83,6 +88,7 @@ pub use platform_address_sync::*; pub use platform_address_types::*; pub use platform_addresses::*; pub use platform_wallet_info::*; +pub use sign_with_mnemonic_resolver::*; pub use spv::*; pub use types::*; pub use utils::*; diff --git a/packages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs b/packages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs new file mode 100644 index 00000000000..168e82f3f49 --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs @@ -0,0 +1,373 @@ +//! `dash_sdk_sign_with_mnemonic_resolver_and_path` — sibling of +//! the lower-level `dash_sdk_sign_with_mnemonic_and_path` in +//! `rs-sdk-ffi/src/signer_simple.rs` that uses a Swift-owned +//! [`MnemonicResolverHandle`](crate::MnemonicResolverHandle) +//! callback to fetch the mnemonic instead of taking it as a raw +//! C-string parameter. +//! +//! Same architectural intent as +//! [`crate::dash_sdk_derive_and_persist_identity_keys`]: keep the +//! Swift caller out of the mnemonic-orchestration loop so the +//! `swift-sdk/CLAUDE.md` "no mnemonic round-tripping" rule is +//! satisfied for the platform-address signing path too. +//! +//! The Swift `KeychainSigner.signPlatformAddressOnDemand` path +//! used to: +//! +//! 1. Pull the mnemonic from Keychain into a Swift `String`. +//! 2. Call the lower-level `dash_sdk_sign_with_mnemonic_and_path` +//! with the raw mnemonic + a derivation path. +//! 3. Get back a signature. +//! +//! With this entry point the same call site can supply a +//! [`MnemonicResolver`](crate::MnemonicResolverHandle) handle +//! whose `resolve` callback is fired by Rust at the moment the +//! mnemonic is needed — Swift's only orchestration is hooking +//! the Keychain read into the resolver impl. +//! +//! # Why this lives in `platform-wallet-ffi` and not `rs-sdk-ffi` +//! +//! The [`MnemonicResolverHandle`](crate::MnemonicResolverHandle) +//! type is defined here. Hoisting it to `rs-sdk-ffi` would force +//! that crate to take on the resolver vtable just so this one +//! sibling FFI could live there too — net zero for the +//! architecture, plus extra cross-crate churn. Symbol names are +//! a flat namespace at the FFI boundary; the C caller doesn't +//! see (or care) which Rust crate produced +//! `dash_sdk_sign_with_mnemonic_resolver_and_path`. + +use std::ffi::{c_void, CStr}; +use std::os::raw::c_char; +use std::str::FromStr; + +use dashcore::secp256k1::Secp256k1; +use key_wallet::bip32::{DerivationPath, ExtendedPrivKey}; +use rs_sdk_ffi::DashSDKNetwork; +use zeroize::Zeroizing; + +use crate::derive_and_persist_callbacks::{ + mnemonic_resolver_result, MnemonicResolverHandle, MNEMONIC_RESOLVER_BUFFER_CAPACITY, +}; +use crate::identity_keys_from_mnemonic::{map_network, parse_mnemonic_any_language}; + +// One-byte error tags. Mirror the shape of +// `signer_simple::SIGN_WITH_MNEMONIC_ERR_*` so call sites already +// familiar with that surface can read the new one without a +// translation table. Keep the same numeric values where possible. +pub const SIGN_WITH_RESOLVER_OK: u8 = 0; +pub const SIGN_WITH_RESOLVER_ERR_NULL_POINTER: u8 = 1; +pub const SIGN_WITH_RESOLVER_ERR_INVALID_UTF8: u8 = 2; +pub const SIGN_WITH_RESOLVER_ERR_INVALID_MNEMONIC: u8 = 3; +pub const SIGN_WITH_RESOLVER_ERR_INVALID_PATH: u8 = 4; +pub const SIGN_WITH_RESOLVER_ERR_DERIVATION: u8 = 5; +pub const SIGN_WITH_RESOLVER_ERR_SIGN: u8 = 6; +pub const SIGN_WITH_RESOLVER_ERR_BUFFER_TOO_SMALL: u8 = 7; +pub const SIGN_WITH_RESOLVER_ERR_UNSUPPORTED_KEY_TYPE: u8 = 8; +/// Resolver callback returned `mnemonic_resolver_result::NOT_FOUND`. +pub const SIGN_WITH_RESOLVER_ERR_RESOLVER_NOT_FOUND: u8 = 9; +/// Resolver callback returned `mnemonic_resolver_result::OTHER`. +pub const SIGN_WITH_RESOLVER_ERR_RESOLVER_FAILED: u8 = 10; + +/// Sign `data` with the ECDSA secp256k1 private key derived from +/// `(mnemonic-via-resolver, derivation_path)`. Mnemonic, seed and +/// derived secret bytes all stay in `Zeroizing` buffers and are +/// scrubbed before this function returns. +/// +/// The resolver callback fires exactly once per call — at the +/// start, when the mnemonic is needed. +/// +/// Same shape as `dash_sdk_sign_with_mnemonic_and_path` +/// (rs-sdk-ffi/src/signer_simple.rs) with the mnemonic argument +/// replaced by `(wallet_id_bytes, mnemonic_resolver_handle)`. The +/// `key_type` parameter exists for parity (only `0` = +/// `ECDSA_SECP256K1` is supported); other key types fail with +/// [`SIGN_WITH_RESOLVER_ERR_UNSUPPORTED_KEY_TYPE`]. +/// +/// Returns `0` on success, `-1` on error. On error, `*out_error` +/// is set to one of the `SIGN_WITH_RESOLVER_ERR_*` tags, +/// `*out_signature_len = 0`, and the first +/// `out_signature_capacity` bytes of `out_signature` are zeroed. +/// +/// # Safety +/// - `wallet_id_bytes` must be valid for 32 readable bytes. +/// - `mnemonic_resolver_handle` must come from +/// [`crate::dash_sdk_mnemonic_resolver_create`]. +/// - `derivation_path_cstr` must be a valid NUL-terminated UTF-8 +/// C-string for the duration of the call. +/// - `data` must point at `data_len` readable bytes (may be zero +/// only if `data_len == 0`). +/// - `out_signature` must point at `out_signature_capacity` +/// writable bytes; `out_signature_len` and `out_error` must be +/// writable. +#[no_mangle] +#[allow(clippy::too_many_arguments)] +pub unsafe extern "C" fn dash_sdk_sign_with_mnemonic_resolver_and_path( + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + wallet_id_bytes: *const u8, + derivation_path_cstr: *const c_char, + data: *const u8, + data_len: usize, + key_type: u8, + network: DashSDKNetwork, + out_signature: *mut u8, + out_signature_capacity: usize, + out_signature_len: *mut usize, + out_error: *mut u8, +) -> i32 { + // Internal helper that scrubs `out_signature` + `*out_signature_len` + // and returns the failure tag. Single exit point for every error path. + let fail = |tag: u8| -> i32 { + if !out_error.is_null() { + *out_error = tag; + } + if !out_signature_len.is_null() { + *out_signature_len = 0; + } + if !out_signature.is_null() && out_signature_capacity > 0 { + std::ptr::write_bytes(out_signature, 0, out_signature_capacity); + } + -1 + }; + + // ---- Argument validation ------------------------------------------------- + if mnemonic_resolver_handle.is_null() + || wallet_id_bytes.is_null() + || derivation_path_cstr.is_null() + || out_signature.is_null() + || out_signature_len.is_null() + || (data.is_null() && data_len > 0) + { + return fail(SIGN_WITH_RESOLVER_ERR_NULL_POINTER); + } + + // ECDSA-only entry point. Anything else is a contract violation. + const ECDSA_SECP256K1: u8 = 0; + if key_type != ECDSA_SECP256K1 { + return fail(SIGN_WITH_RESOLVER_ERR_UNSUPPORTED_KEY_TYPE); + } + + // ---- Resolve mnemonic ---------------------------------------------------- + let mut mnemonic_buf: Zeroizing<[u8; MNEMONIC_RESOLVER_BUFFER_CAPACITY]> = + Zeroizing::new([0u8; MNEMONIC_RESOLVER_BUFFER_CAPACITY]); + let mut mnemonic_len: usize = 0; + + let resolver = &*mnemonic_resolver_handle; + let resolver_vtable = &*resolver.vtable; + let rc = (resolver_vtable.resolve)( + resolver.ctx as *const c_void, + wallet_id_bytes, + mnemonic_buf.as_mut_ptr() as *mut c_char, + MNEMONIC_RESOLVER_BUFFER_CAPACITY, + &mut mnemonic_len, + ); + match rc { + x if x == mnemonic_resolver_result::SUCCESS => {} + x if x == mnemonic_resolver_result::NOT_FOUND => { + return fail(SIGN_WITH_RESOLVER_ERR_RESOLVER_NOT_FOUND); + } + x if x == mnemonic_resolver_result::BUFFER_TOO_SMALL => { + return fail(SIGN_WITH_RESOLVER_ERR_BUFFER_TOO_SMALL); + } + _ => return fail(SIGN_WITH_RESOLVER_ERR_RESOLVER_FAILED), + } + if mnemonic_len == 0 || mnemonic_len > MNEMONIC_RESOLVER_BUFFER_CAPACITY { + return fail(SIGN_WITH_RESOLVER_ERR_RESOLVER_FAILED); + } + + // Parse mnemonic. UTF-8 validation runs on the prefix only — + // we never construct a `String` (Swift's String can't be + // zeroized; ours can). + let mnemonic_str = match std::str::from_utf8(&mnemonic_buf[..mnemonic_len]) { + Ok(s) => s, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_INVALID_UTF8), + }; + let mnemonic = match parse_mnemonic_any_language(mnemonic_str) { + Ok(m) => m, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_INVALID_MNEMONIC), + }; + + // ---- Derive seed + derivation path -------------------------------------- + let seed: Zeroizing<[u8; 64]> = Zeroizing::new(mnemonic.to_seed("")); + drop(mnemonic); + + let path_str = match CStr::from_ptr(derivation_path_cstr).to_str() { + Ok(s) => s, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_INVALID_UTF8), + }; + let path = match DerivationPath::from_str(path_str) { + Ok(p) => p, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_INVALID_PATH), + }; + + let kw_network = map_network(network); + let master = match ExtendedPrivKey::new_master(kw_network, seed.as_ref()) { + Ok(m) => m, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_DERIVATION), + }; + let secp = Secp256k1::new(); + let derived = match master.derive_priv(&secp, &path) { + Ok(d) => d, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_DERIVATION), + }; + let secret_bytes: Zeroizing<[u8; 32]> = Zeroizing::new(derived.private_key.secret_bytes()); + + // ---- Sign --------------------------------------------------------------- + let data_slice: &[u8] = if data_len == 0 { + &[] + } else { + std::slice::from_raw_parts(data, data_len) + }; + let sig_array = match dash_sdk::dpp::dashcore::signer::sign(data_slice, secret_bytes.as_ref()) { + Ok(s) => s, + Err(_) => return fail(SIGN_WITH_RESOLVER_ERR_SIGN), + }; + + if sig_array.len() > out_signature_capacity { + return fail(SIGN_WITH_RESOLVER_ERR_BUFFER_TOO_SMALL); + } + + std::ptr::copy_nonoverlapping(sig_array.as_ptr(), out_signature, sig_array.len()); + *out_signature_len = sig_array.len(); + if !out_error.is_null() { + *out_error = SIGN_WITH_RESOLVER_OK; + } + 0 +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::derive_and_persist_callbacks::{ + dash_sdk_mnemonic_resolver_create, dash_sdk_mnemonic_resolver_destroy, + }; + use std::ffi::CString; + + /// English BIP-39 test vector (all-zero entropy). + const ENGLISH_PHRASE: &str = + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + + unsafe extern "C" fn english_resolve( + _ctx: *const c_void, + _wallet_id_bytes: *const u8, + out_buf: *mut c_char, + out_capacity: usize, + out_len: *mut usize, + ) -> i32 { + let phrase = ENGLISH_PHRASE.as_bytes(); + if phrase.len() + 1 > out_capacity { + return mnemonic_resolver_result::BUFFER_TOO_SMALL; + } + std::ptr::copy_nonoverlapping(phrase.as_ptr() as *const c_char, out_buf, phrase.len()); + *out_buf.add(phrase.len()) = 0; + *out_len = phrase.len(); + mnemonic_resolver_result::SUCCESS + } + + unsafe extern "C" fn missing_resolve( + _ctx: *const c_void, + _wallet_id_bytes: *const u8, + _out_buf: *mut c_char, + _out_capacity: usize, + _out_len: *mut usize, + ) -> i32 { + mnemonic_resolver_result::NOT_FOUND + } + + unsafe extern "C" fn noop_destroy(_ctx: *mut c_void) {} + + fn make_resolver( + cb: crate::derive_and_persist_callbacks::MnemonicResolveCallback, + ) -> *mut MnemonicResolverHandle { + unsafe { dash_sdk_mnemonic_resolver_create(std::ptr::null_mut(), cb, noop_destroy) } + } + + #[test] + fn happy_path_signs_and_returns_signature() { + let resolver = make_resolver(english_resolve); + let path = CString::new("m/9'/1'/5'/0'/0'/0'/0'").unwrap(); + let wallet_id = [0u8; 32]; + let data = b"hello"; + let mut sig_buf = [0u8; 128]; + let mut sig_len: usize = 0; + let mut err: u8 = 0; + let rc = unsafe { + dash_sdk_sign_with_mnemonic_resolver_and_path( + resolver, + wallet_id.as_ptr(), + path.as_ptr(), + data.as_ptr(), + data.len(), + 0, // ECDSA_SECP256K1 + DashSDKNetwork::SDKTestnet, + sig_buf.as_mut_ptr(), + sig_buf.len(), + &mut sig_len, + &mut err, + ) + }; + assert_eq!(rc, 0); + assert_eq!(err, SIGN_WITH_RESOLVER_OK); + // dashcore compact-recoverable secp256k1 signature is 65 bytes. + assert_eq!(sig_len, 65); + unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; + } + + #[test] + fn missing_resolver_surfaces_not_found_tag() { + let resolver = make_resolver(missing_resolve); + let path = CString::new("m/9'/1'/5'/0'/0'/0'/0'").unwrap(); + let wallet_id = [0u8; 32]; + let data = b"x"; + let mut sig_buf = [0u8; 128]; + let mut sig_len: usize = 0; + let mut err: u8 = 0; + let rc = unsafe { + dash_sdk_sign_with_mnemonic_resolver_and_path( + resolver, + wallet_id.as_ptr(), + path.as_ptr(), + data.as_ptr(), + data.len(), + 0, + DashSDKNetwork::SDKTestnet, + sig_buf.as_mut_ptr(), + sig_buf.len(), + &mut sig_len, + &mut err, + ) + }; + assert_eq!(rc, -1); + assert_eq!(err, SIGN_WITH_RESOLVER_ERR_RESOLVER_NOT_FOUND); + assert_eq!(sig_len, 0); + unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; + } + + #[test] + fn rejects_unsupported_key_type() { + let resolver = make_resolver(english_resolve); + let path = CString::new("m/9'/1'/5'/0'/0'/0'/0'").unwrap(); + let wallet_id = [0u8; 32]; + let data = b"x"; + let mut sig_buf = [0u8; 128]; + let mut sig_len: usize = 0; + let mut err: u8 = 0; + let rc = unsafe { + dash_sdk_sign_with_mnemonic_resolver_and_path( + resolver, + wallet_id.as_ptr(), + path.as_ptr(), + data.as_ptr(), + data.len(), + 1, // BLS12_381 — not supported + DashSDKNetwork::SDKTestnet, + sig_buf.as_mut_ptr(), + sig_buf.len(), + &mut sig_len, + &mut err, + ) + }; + assert_eq!(rc, -1); + assert_eq!(err, SIGN_WITH_RESOLVER_ERR_UNSUPPORTED_KEY_TYPE); + unsafe { dash_sdk_mnemonic_resolver_destroy(resolver) }; + } +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index 70e5d9d4cac..a70dd7c2cf0 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -139,6 +139,14 @@ public final class KeychainSigner: Signer, @unchecked Sendable { /// constructed there. private let queue: DispatchQueue + /// Resolver handle the platform-address signing path uses to + /// fetch the wallet's mnemonic out of Keychain. The mnemonic + /// flows IN to a Rust-owned `Zeroizing` buffer through the + /// resolver's `resolve` callback; it never lives in a Swift + /// `String` outside the trampoline's stack frame. Per + /// swift-sdk/CLAUDE.md "no mnemonic round-tripping". + private let mnemonicResolver: MnemonicResolver + /// Raw pointer to the FFI signer handle. Boxed by Rust and freed /// in `deinit`. private var handlePtr: UnsafeMutablePointer! @@ -162,6 +170,10 @@ public final class KeychainSigner: Signer, @unchecked Sendable { self.network = network self.keychain = keychain self.queue = DispatchQueue(label: "org.dashfoundation.swiftdashsdk.KeychainSigner") + // One resolver per signer instance. Cheap to keep around — + // it's just an opaque handle + a Swift-side `WalletStorage` + // reference. Used by the platform-address signing branch. + self.mnemonicResolver = MnemonicResolver() // Hand Rust an opaque `+1`-retained pointer to self. The // matching release happens in `keychainSignerDestroyTrampoline` @@ -376,12 +388,19 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } /// One-shot derive-and-sign for the `key_type == 0xFF` branch. - /// Pulls mnemonic from Keychain + derivation path from SwiftData, - /// then calls `dash_sdk_sign_with_mnemonic_and_path` which - /// derives the ECDSA key inside Rust, signs `data` with it, and - /// zeroes both the seed and the derived key buffer before - /// returning. The derived bytes never cross the FFI back to - /// Swift — only the signature does. + /// Resolves the SwiftData `(walletId, derivationPath)` row and + /// hands the Rust `dash_sdk_sign_with_mnemonic_resolver_and_path` + /// FFI the wallet id + the resolver handle. The Rust side + /// fires the resolver callback at the moment the mnemonic is + /// needed — the mnemonic is copied into a Rust-owned + /// `Zeroizing` buffer and the derived ECDSA key bytes never + /// cross the FFI back to Swift. Only the signature does. + /// + /// Per swift-sdk/CLAUDE.md "no mnemonic round-tripping": the + /// Swift caller does NOT pull the mnemonic into a `String` + /// and hand it to a stateless FFI; the resolver callback is + /// the only place the mnemonic crosses any boundary, and its + /// lifetime is bounded by the trampoline's stack frame. fileprivate func signPlatformAddressOnDemand( addressHash: Data, keyType: UInt8, @@ -390,35 +409,20 @@ public final class KeychainSigner: Signer, @unchecked Sendable { let hashHex = addressHash.map { String(format: "%02x", $0) }.joined() // 1. Look up `(walletId, derivationPath)` on a private - // background ModelContext (off the main actor). + // background ModelContext (off the main actor). Mnemonic + // fetch is now Rust's job (via the resolver callback), + // not Swift's. guard let ctx = resolvePlatformAddressContext(addressHash: addressHash) else { return .failure(.platformAddressNotFound(addressHashHex: hashHex)) } - // 2. Pull mnemonic from Keychain. `WalletStorage` throws on - // miss; map to our typed error so the trampoline can - // surface a precise message. - let mnemonic: String - do { - mnemonic = try WalletStorage().retrieveMnemonic(for: ctx.walletId) - } catch { - let walletHex = ctx.walletId.map { String(format: "%02x", $0) }.joined() - return .failure(.mnemonicMissing(walletIdHex: walletHex)) - } - - // 3. One-shot FFI: derive → sign → zeroize. - // Buffer is sized generously (128B) — ECDSA compact - // recoverable signatures are 65 bytes; the cap leaves - // room for any future signature-shape additions through - // this entry point without an ABI change. + // Buffer sized generously (128B) — ECDSA compact-recoverable + // signatures are 65 bytes; the cap leaves room for future + // signature-shape additions without an ABI change. Defer- + // scrubbed below regardless of success/failure. var sigBuf = [UInt8](repeating: 0, count: 128) var sigLen: Int = 0 var errTag: UInt8 = 0 - - // Defensive scrub of the local `sigBuf` regardless of - // success/failure. Swift can't truly zero a `String`, but - // `mnemonic` falls out of scope at function exit which is - // the best we can do without a native-Swift mnemonic type. defer { sigBuf.withUnsafeMutableBufferPointer { ptr in if let base = ptr.baseAddress { @@ -431,25 +435,24 @@ public final class KeychainSigner: Signer, @unchecked Sendable { // `0xFF` value the trampoline received is the dispatch tag // (`platformAddressHashKeyType`), used only to route to this // branch — it is NOT a `KeyType` discriminant. Hardcode the - // real key type here; passing the dispatch tag through to - // `dash_sdk_sign_with_mnemonic_and_path` (which is ECDSA-only) - // would fail with `SIGN_WITH_MNEMONIC_ERR_UNSUPPORTED_KEY_TYPE`. + // real key type here. let ecdsaSecp256k1KeyType: UInt8 = 0 - let rc = mnemonic.withCString { mPtr -> Int32 in - ctx.derivationPath.withCString { pPtr -> Int32 in - data.withUnsafeBytes { dataRaw -> Int32 in - sigBuf.withUnsafeMutableBufferPointer { bufPtr -> Int32 in + let rc = ctx.walletId.withUnsafeBytes { walletBytes -> Int32 in + let walletPtr = walletBytes.bindMemory(to: UInt8.self).baseAddress + return ctx.derivationPath.withCString { pPtr -> Int32 in + return data.withUnsafeBytes { dataRaw -> Int32 in + return sigBuf.withUnsafeMutableBufferPointer { bufPtr -> Int32 in let dataBase = dataRaw.bindMemory(to: UInt8.self).baseAddress - return dash_sdk_sign_with_mnemonic_and_path( - mPtr, - nil, // no BIP-39 passphrase yet (matches the rest of the SDK) + return dash_sdk_sign_with_mnemonic_resolver_and_path( + self.mnemonicResolver.handle, + walletPtr, pPtr, dataBase, - UInt(dataRaw.count), + dataRaw.count, ecdsaSecp256k1KeyType, self.network, bufPtr.baseAddress, - UInt(bufPtr.count), + bufPtr.count, &sigLen, &errTag ) @@ -459,6 +462,14 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } guard rc == 0 else { + // `RESOLVER_NOT_FOUND` (9) is the recoverable + // user-visible case — surface it through the existing + // typed `mnemonicMissing` error so the UI message + // stays specific. + if errTag == SignWithMnemonicResolverError.resolverNotFound.rawValue { + let walletHex = ctx.walletId.map { String(format: "%02x", $0) }.joined() + return .failure(.mnemonicMissing(walletIdHex: walletHex)) + } return .failure(.signWithMnemonicFailed(tag: errTag)) } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift new file mode 100644 index 00000000000..9294775655d --- /dev/null +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift @@ -0,0 +1,288 @@ +import DashSDKFFI +import Foundation + +// MARK: - MnemonicResolver + +/// Swift bridge backing the Rust-side `MnemonicResolverHandle`. +/// +/// The Rust derivation loop in +/// `dash_sdk_derive_and_persist_identity_keys` calls back into +/// Swift via this resolver to fetch the BIP-39 mnemonic for the +/// wallet whose identity keys it's deriving. The mnemonic is +/// copied directly into a Rust-owned `Zeroizing` stack buffer; it +/// never round-trips back to Swift after this single read, and +/// the Swift `String` from `WalletStorage.retrieveMnemonic` falls +/// out of scope at the end of the trampoline. +/// +/// # Lifetime +/// +/// Init allocates a `MnemonicResolverHandle*` via +/// `dash_sdk_mnemonic_resolver_create`, handing Rust an +/// `Unmanaged.passRetained(self)` pointer. `deinit` calls +/// `dash_sdk_mnemonic_resolver_destroy`, which fires the +/// destructor trampoline that releases the +1 retain. The handle +/// is safe to pass to FFI calls for the entire lifetime of this +/// instance. +public final class MnemonicResolver: @unchecked Sendable { + /// Owned for the lifetime of this object. Pass this to + /// `dash_sdk_derive_and_persist_identity_keys`. + public var handle: UnsafeMutablePointer? { + handlePtr + } + + private let storage: WalletStorage + private var handlePtr: UnsafeMutablePointer? + + /// - Parameter storage: source for the BIP-39 mnemonic per + /// `walletId`. Defaults to a fresh `WalletStorage()` so + /// tests can substitute a mock. + public init(storage: WalletStorage = WalletStorage()) { + self.storage = storage + + let ctx = Unmanaged.passRetained(self).toOpaque() + self.handlePtr = dash_sdk_mnemonic_resolver_create( + ctx, + mnemonicResolverResolveTrampoline, + mnemonicResolverDestroyTrampoline + ) + precondition( + handlePtr != nil, + "dash_sdk_mnemonic_resolver_create returned NULL" + ) + } + + deinit { + if let handlePtr { + dash_sdk_mnemonic_resolver_destroy(handlePtr) + } + } + + /// Trampoline-callable internals. + fileprivate func resolve( + walletId: Data, + outBuffer: UnsafeMutablePointer, + outCapacity: Int, + outLen: UnsafeMutablePointer + ) -> MnemonicResolverResult { + let mnemonic: String + do { + mnemonic = try storage.retrieveMnemonic(for: walletId) + } catch { + return .notFound + } + + // `withCString` materializes a null-terminated UTF-8 byte + // sequence whose lifetime ends with the closure. We copy + // (excluding the trailing NUL) into the Rust-owned + // buffer; the source bytes drop with the Swift `String` + // at the end of `resolve`. + return mnemonic.withCString { srcPtr -> MnemonicResolverResult in + let mnemonicLen = strlen(srcPtr) + // Need room for the data plus a trailing NUL byte. + guard mnemonicLen + 1 <= outCapacity else { + return .bufferTooSmall + } + outBuffer.update(from: srcPtr, count: mnemonicLen) + // Explicit NUL terminator — defensive, the Rust side + // works off `out_len` not strlen but matching the + // wire contract is cheap insurance. + (outBuffer + mnemonicLen).pointee = 0 + outLen.pointee = mnemonicLen + return .success + } + } +} + +// MARK: - MnemonicResolver C trampolines +// +// These run on whatever thread the Rust derivation loop is +// executing on (typically a background queue from the iOS +// caller). `WalletStorage.retrieveMnemonic` is thread-safe; no +// further synchronization is required. + +private func mnemonicResolverResolveTrampoline( + ctx: UnsafeRawPointer?, + walletIdBytes: UnsafePointer?, + outBuffer: UnsafeMutablePointer?, + outCapacity: Int, + outLen: UnsafeMutablePointer? +) -> Int32 { + guard let ctx, let walletIdBytes, let outBuffer, let outLen else { + return MnemonicResolverResult.other.rawValue + } + let resolver = Unmanaged.fromOpaque(ctx).takeUnretainedValue() + let walletId = Data(bytes: walletIdBytes, count: 32) + let result = resolver.resolve( + walletId: walletId, + outBuffer: outBuffer, + outCapacity: outCapacity, + outLen: outLen + ) + return result.rawValue +} + +private func mnemonicResolverDestroyTrampoline(ctx: UnsafeMutableRawPointer?) { + guard let ctx else { return } + Unmanaged.fromOpaque(ctx).release() +} + +// MARK: - IdentityKeyPersister + +/// Swift bridge backing the Rust-side +/// `IdentityKeyPersisterHandle`. The Rust derivation loop calls +/// back here once per derived key with a [`PersistKeyArgs`] +/// payload; this class translates that into a +/// `KeychainManager.storeIdentityPrivateKey(...)` call. +/// +/// Per-key DPP metadata (key_type, purpose, security_level) is +/// computed entirely on the Rust side — Swift just stamps +/// whatever bytes it's told into Keychain. This is the inverse of +/// the prior shape, where Swift hardcoded `keyId 0 -> MASTER, +/// else HIGH` plus the DPP discriminant bytes. +public final class IdentityKeyPersister: @unchecked Sendable { + /// Per-key metadata captured as a side effect of each persist + /// callback firing during a `dash_sdk_derive_and_persist_identity_keys` + /// call. Read this AFTER the FFI returns to recover the + /// `(keyType, purpose, securityLevel)` triple Rust decided per + /// key — useful for building `IdentityPubkey` rows without + /// recreating the MASTER-vs-HIGH policy on the Swift side. + public struct PersistedKeyMetadata: Sendable { + public let identityIndex: UInt32 + public let keyId: UInt32 + public let keyIndex: UInt32 + public let derivationPath: String + public let publicKey: Data + public let publicKeyHash: Data + public let keyType: UInt8 + public let purpose: UInt8 + public let securityLevel: UInt8 + } + + public var handle: UnsafeMutablePointer? { + handlePtr + } + + /// Snapshot of every key the persister wrote during the most + /// recent FFI call. Indexed in callback-fire order (= the + /// derivation order Rust used: key_index 0..key_count). Reset + /// implicitly per `IdentityKeyPersister` instance — typical + /// usage constructs a fresh persister per derive call so the + /// list always describes one logical batch. + public var persistedKeys: [PersistedKeyMetadata] { + // Trampolines fire synchronously on the same thread the + // FFI call was invoked on, so a plain unsynchronized read + // is safe at the natural use site (after FFI returns). + // Marked @unchecked Sendable for the same reason. + _persistedKeys + } + private var _persistedKeys: [PersistedKeyMetadata] = [] + + private let keychain: KeychainManager + private var handlePtr: UnsafeMutablePointer? + + public init(keychain: KeychainManager = .shared) { + self.keychain = keychain + + let ctx = Unmanaged.passRetained(self).toOpaque() + self.handlePtr = dash_sdk_identity_key_persister_create( + ctx, + identityKeyPersisterPersistTrampoline, + identityKeyPersisterDestroyTrampoline + ) + precondition( + handlePtr != nil, + "dash_sdk_identity_key_persister_create returned NULL" + ) + } + + deinit { + if let handlePtr { + dash_sdk_identity_key_persister_destroy(handlePtr) + } + } + + /// Trampoline-callable internal. Returns `false` to abort + /// the rest of the Rust derivation loop with an + /// `ErrorWalletOperation`; returns `true` to let it continue. + fileprivate func persist(args: UnsafePointer) -> Bool { + let a = args.pointee + guard + let walletIdPtr = a.wallet_id_bytes, + let pathPtr = a.derivation_path_cstr, + let pubKeyPtr = a.public_key_bytes, + let pubHashPtr = a.public_key_hash_bytes, + let privKeyPtr = a.private_key_bytes + else { + return false + } + + let walletIdBytes = Data(bytes: walletIdPtr, count: 32) + let walletIdHex = walletIdBytes.map { String(format: "%02x", $0) }.joined() + let derivationPath = String(cString: pathPtr) + let publicKeyData = Data(bytes: pubKeyPtr, count: a.public_key_len) + let publicKeyHex = publicKeyData.map { String(format: "%02x", $0) }.joined() + let publicKeyHashData = Data(bytes: pubHashPtr, count: 20) + let publicKeyHashHex = publicKeyHashData + .map { String(format: "%02x", $0) } + .joined() + let privateKeyData = Data(bytes: privKeyPtr, count: 32) + + // Identity-id is unknown pre-registration — Rust will + // recompute it from the input addresses at submit time. + // Use the marker `pending` so the keychain explorer makes + // it obvious which rows are pre-registered slots. + let metadata = KeychainManager.IdentityPrivateKeyMetadata( + identityId: "pending", + keyId: a.key_id, + walletId: walletIdHex, + identityIndex: a.identity_index, + keyIndex: a.key_index, + derivationPath: derivationPath, + publicKey: publicKeyHex, + publicKeyHash: publicKeyHashHex, + keyType: a.key_type, + purpose: a.purpose, + securityLevel: a.security_level + ) + + let stored = keychain.storeIdentityPrivateKey( + privateKeyData, + derivationPath: derivationPath, + metadata: metadata + ) + if stored == nil { + return false + } + + // Capture the per-key metadata so the caller can build + // `IdentityPubkey` rows from it without re-deciding the + // MASTER-vs-HIGH policy that already lives in Rust. + _persistedKeys.append(PersistedKeyMetadata( + identityIndex: a.identity_index, + keyId: a.key_id, + keyIndex: a.key_index, + derivationPath: derivationPath, + publicKey: publicKeyData, + publicKeyHash: publicKeyHashData, + keyType: a.key_type, + purpose: a.purpose, + securityLevel: a.security_level + )) + return true + } +} + +private func identityKeyPersisterPersistTrampoline( + ctx: UnsafeRawPointer?, + args: UnsafeRawPointer? +) -> UInt8 { + guard let ctx, let args else { return PERSIST_KEY_FAILURE } + let persister = Unmanaged.fromOpaque(ctx).takeUnretainedValue() + let typedArgs = args.assumingMemoryBound(to: PersistKeyArgs.self) + return persister.persist(args: typedArgs) ? PERSIST_KEY_SUCCESS : PERSIST_KEY_FAILURE +} + +private func identityKeyPersisterDestroyTrampoline(ctx: UnsafeMutableRawPointer?) { + guard let ctx else { return } + Unmanaged.fromOpaque(ctx).release() +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index 1bef67ed2d2..c950e515026 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -650,25 +650,30 @@ extension ManagedPlatformWallet { /// fallback path. The matching SwiftData rows are written by /// the persister callback once registration succeeds. /// - /// All derivation happens entirely on the Rust side via - /// `dash_sdk_derive_identity_keys_from_mnemonic`. Swift's role is - /// purely (a) pulling the mnemonic out of Keychain and (b) writing - /// the resulting private keys back into Keychain — the only - /// operations Rust cannot perform from its side, per - /// `swift-sdk/CLAUDE.md`. + /// # Architecture: derivation loop lives in Rust /// - /// # Why this routes through the mnemonic-driven FFI + /// All derivation, the per-key MASTER-vs-HIGH policy, and the + /// DPP discriminant bytes (`KeyType`, `Purpose`, + /// `SecurityLevel`) live on the Rust side. Swift hands the + /// derivation FFI two callback handles — a + /// [`MnemonicResolver`] (Rust → Swift "fetch the BIP-39 + /// mnemonic for `walletId`") and an + /// [`IdentityKeyPersister`] (Rust → Swift "save this derived + /// key, here's its metadata") — and Rust runs the loop + /// without Swift orchestrating it. Closes the swift-sdk/CLAUDE.md + /// "no mnemonic round-tripping" rule that the prior shape + /// (Swift pulls mnemonic, calls a stateless derivation FFI, + /// loops over the rows writing each to Keychain) violated. /// - /// The previous implementation called the wallet-handle variant - /// (`platform_wallet_derive_identity_keys_for_index`), which - /// required Rust to have an in-process xpriv for this wallet. That - /// works for wallets created in the current process but fails with - /// `"Cannot derive private keys from watch-only wallet"` for - /// wallets restored from SwiftData persistence (their seed lives - /// in Keychain, not in the `WalletManager`). Pulling the mnemonic - /// from Keychain per call and handing it to a stateless derivation - /// FFI works for every wallet shape regardless of how it was - /// loaded into the process. + /// # Why this works for watch-only wallets + /// + /// The previous implementation that routed through the + /// wallet-handle variant + /// (`platform_wallet_derive_identity_keys_for_index`) failed + /// for restored watch-only wallets because Rust had no + /// in-process xpriv loaded for them. Routing through the + /// resolver callback works regardless of wallet shape — the + /// resolver pulls the mnemonic from iOS Keychain on demand. /// /// - Parameters: /// - identityIndex: The identity index that will be passed to @@ -681,50 +686,53 @@ extension ManagedPlatformWallet { /// on. Selects the DIP-9 coin-type slot in the derivation /// paths AND the WIF version byte. Must match the network of /// the SDK / `KeychainSigner` used at registration time. - /// - walletIdHex: Hex-encoded wallet id used in the keychain - /// metadata blob — surfaced by the keychain-explorer view - /// and by `PersistentPublicKey` later. Should match the - /// wallet's `walletId` (same value the Rust persister - /// callback would write). /// - keychain: Defaults to `KeychainManager.shared`. /// - storage: `WalletStorage` instance used to read the BIP-39 /// mnemonic from Keychain. Defaults to a fresh /// `WalletStorage()` — overridable for tests. - /// - Returns: The previews that were just persisted (in the same - /// order as `keyCount`), so callers can hold on to the pubkey - /// data if they need to render anything else. + /// - Returns: One [`IdentityPubkey`] per derived/persisted key, + /// ready to thread directly into + /// `registerIdentityFromAddresses(...identityPubkeys:...)`. + /// The `(keyType, purpose, securityLevel)` triple on each row + /// is whatever Rust decided — the caller does NOT recreate + /// the MASTER-vs-HIGH policy. @discardableResult public func prePersistIdentityKeysForRegistration( identityIndex: UInt32, keyCount: UInt32, network: DashSDKNetwork, - walletIdHex: String, keychain: KeychainManager = .shared, storage: WalletStorage = WalletStorage() - ) throws -> [IdentityRegistrationKeyPreview] { + ) throws -> [IdentityPubkey] { guard keyCount > 0 else { return [] } - // Pull the BIP-39 mnemonic out of Keychain. `String` can't be - // truly zeroed in Swift, but we keep the local lifetime as - // narrow as possible — only this function holds a reference, - // and the FFI call inside copies the bytes onto the Rust side - // (where they ARE held in `Zeroizing` and scrubbed at function - // exit). - let mnemonic = try storage.retrieveMnemonic(for: self.walletId) + // Single-FFI derivation + persist. The mnemonic is pulled + // from Keychain by the resolver callback (Rust → Swift), + // each derived key is written by the persister callback + // (Rust → Swift), and only pubkeys + paths flow back as + // the function's return value. Per swift-sdk/CLAUDE.md no + // pipeline orchestration crosses the FFI boundary; the + // mnemonic never lives in a Swift `String` outside the + // resolver trampoline's stack frame, and the 32-byte + // private scalars never leave the Rust loop. + let resolver = MnemonicResolver(storage: storage) + let persister = IdentityKeyPersister(keychain: keychain) var out = identityRegistrationKeyDerivationsFFIEmpty() var error = PlatformWalletFFIError() - let result = mnemonic.withCString { mPtr -> PlatformWalletFFIResult in - // No BIP-39 passphrase is supported by the rest of the - // SDK yet (mirrors `KeychainSigner.swift`'s own - // `dash_sdk_sign_with_mnemonic_and_path` call site). - // Pass `nil` to mean "empty passphrase". - dash_sdk_derive_identity_keys_from_mnemonic( - mPtr, - nil, + + // `walletId` is `Data`; bind into a 32-byte UInt8 pointer + // so Rust receives a stable address for the duration of + // the FFI call. + let result = self.walletId.withUnsafeBytes { walletBytes -> PlatformWalletFFIResult in + let walletPtr = walletBytes.bindMemory(to: UInt8.self).baseAddress + return dash_sdk_derive_and_persist_identity_keys( network, + walletPtr, identityIndex, keyCount, + resolver.handle, + persister.handle, &out, &error ) @@ -737,81 +745,40 @@ extension ManagedPlatformWallet { guard let base = out.items, out.count > 0 else { return [] } - var persisted: [IdentityRegistrationKeyPreview] = [] - persisted.reserveCapacity(out.count) + // Build `IdentityPubkey` rows from (a) the FFI's pubkey + // bytes and (b) the persister's captured per-key metadata + // — both indexed in derivation order. Rust decided + // (keyType, purpose, securityLevel); Swift just maps each + // discriminant byte back into its strongly-typed enum. + let captured = persister.persistedKeys + precondition( + captured.count == out.count, + "persister callback fired \(captured.count) times but FFI returned \(out.count) pubkey rows" + ) + var pubkeys: [IdentityPubkey] = [] + pubkeys.reserveCapacity(out.count) for i in 0.. 0 { pubData = Data(bytes: pubPtr, count: row.public_key_len) - pubHex = pubData.map { String(format: "%02x", $0) }.joined() } else { pubData = Data() - pubHex = "" } - - var pkTuple = row.private_key_bytes - let pkData = withUnsafeBytes(of: &pkTuple) { Data($0) } - - // Identity-id is unknown pre-registration (Rust - // recomputes it from the input addresses). Use the - // marker `pending` so the keychain explorer makes it - // obvious which rows are pre-registered slots. - // - // Discriminant convention matches `CreateIdentityView`'s - // `IdentityPubkey` mapping (and the upstream Rust-side - // policy that lived inside `register_identity` before it - // moved to Swift): - // - keyId 0 → MASTER, AUTHENTICATION, ECDSA_SECP256K1 - // - keyId > 0 → HIGH, AUTHENTICATION, ECDSA_SECP256K1 - // The bytes used here (`0` / `1` / `0`) are the - // canonical DPP `repr(u8)` discriminants, the same ones - // every other FFI surface in this SDK speaks. - let pubHashHex = KeychainManager.computePublicKeyHashHex(pubData) - // CreateIdentityView's enum mapping uses .high for - // non-master rows; the DPP `SecurityLevel` discriminant - // for HIGH is 2 (0=MASTER, 1=CRITICAL, 2=HIGH, 3=MEDIUM). - // Mirror that here so the Keychain row stamps the same - // security level the pubkey row actually submitted to - // the network carries. - let secLevelByte: UInt8 = i == 0 ? 0 /* MASTER */ : 2 /* HIGH */ - let metadata = KeychainManager.IdentityPrivateKeyMetadata( - identityId: "pending", - keyId: UInt32(i), - walletId: walletIdHex, - identityIndex: identityIndex, - keyIndex: UInt32(i), - derivationPath: path, - publicKey: pubHex, - publicKeyHash: pubHashHex, - keyType: 0, // ECDSA_SECP256K1 - purpose: 0, // AUTHENTICATION - securityLevel: secLevelByte - ) - _ = keychain.storeIdentityPrivateKey( - pkData, - derivationPath: path, - metadata: metadata - ) - - persisted.append( - IdentityRegistrationKeyPreview( - identityIndex: identityIndex, - derivationPath: path, - publicKeyData: pubData, - publicKeyHex: pubHex, - privateKeyWIF: wif, - privateKeyData: pkData + let meta = captured[i] + pubkeys.append( + IdentityPubkey( + keyId: meta.keyId, + keyType: KeyType(rawValue: meta.keyType) ?? .ecdsaSecp256k1, + purpose: KeyPurpose(rawValue: meta.purpose) ?? .authentication, + securityLevel: SecurityLevel(rawValue: meta.securityLevel) ?? .high, + pubkeyBytes: pubData, + readOnly: false ) ) } - - return persisted + return pubkeys } // ---------------------------------------------------------------- diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift index 44ad578f231..71354869c1a 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift @@ -1330,3 +1330,192 @@ func dash_sdk_derive_identity_keys_from_mnemonic( func dash_sdk_derive_identity_keys_from_mnemonic_free( _ rows: UnsafeMutablePointer ) + +// MARK: - Derive-and-persist callback handles +// +// Used by `dash_sdk_derive_and_persist_identity_keys` (below). The +// Swift side hands the Rust derivation loop two opaque callback +// handles — one for fetching the BIP-39 mnemonic out of Keychain, +// one for writing each derived key back into Keychain — and the +// Rust loop owns the orchestration. Closes the +// "no mnemonic round-tripping" rule that ManagedPlatformWallet's +// `prePersistIdentityKeysForRegistration` previously violated. + +/// Opaque Rust-side handle to a Swift-owned mnemonic resolver. +/// Allocated via `dash_sdk_mnemonic_resolver_create`, freed via +/// `dash_sdk_mnemonic_resolver_destroy`. The Rust struct itself +/// is private; Swift only ever holds the pointer. +public struct MnemonicResolverHandle {} + +/// Opaque Rust-side handle to a Swift-owned identity-key persister. +public struct IdentityKeyPersisterHandle {} + +/// Mirrors the Rust `mnemonic_resolver_result` constants in +/// `derive_and_persist_callbacks.rs`. Used as the return value of +/// the resolver callback so the Rust derivation loop can +/// distinguish "buffer too small" (a programmer error) from +/// "wallet has no mnemonic stored" (a recoverable user-visible +/// error). +enum MnemonicResolverResult: Int32 { + case success = 0 + case notFound = 1 + case bufferTooSmall = 2 + case other = 3 +} + +/// Buffer capacity (bytes, excluding trailing NUL) the resolver +/// callback is given to write the mnemonic into. Mirrors the Rust +/// `MNEMONIC_RESOLVER_BUFFER_CAPACITY` constant. +let MNEMONIC_RESOLVER_BUFFER_CAPACITY: Int = 1024 + +/// Function pointer type for the mnemonic-resolve callback. +/// Returns one of [`MnemonicResolverResult`]'s raw values. +typealias MnemonicResolveCallback = @convention(c) ( + _ ctx: UnsafeRawPointer?, + _ wallet_id_bytes: UnsafePointer?, + _ out_mnemonic_utf8: UnsafeMutablePointer?, + _ out_capacity: Int, + _ out_len: UnsafeMutablePointer? +) -> Int32 + +/// Mirrors the Rust `PersistKeyArgs` `#[repr(C)]` struct. Pointer- +/// based per-call payload for the identity-key persister callback. +/// Field order, sizes, and trailing-byte alignment match the Rust +/// definition. +struct PersistKeyArgs { + var wallet_id_bytes: UnsafePointer? + var identity_index: UInt32 + var key_id: UInt32 + var key_index: UInt32 + var derivation_path_cstr: UnsafePointer? + var public_key_bytes: UnsafePointer? + var public_key_len: Int + var public_key_hash_bytes: UnsafePointer? + var private_key_bytes: UnsafePointer? + var key_type: UInt8 + var purpose: UInt8 + var security_level: UInt8 +} + +/// Function pointer type for the per-key persist callback. +/// Returns [`PERSIST_KEY_SUCCESS`] on a successful Keychain write, +/// [`PERSIST_KEY_FAILURE`] to abort the rest of the Rust derivation +/// loop with an `ErrorWalletOperation`. +/// +/// The args parameter is `UnsafeRawPointer?` rather than the more +/// natural `UnsafePointer?` because Swift's +/// `@convention(c)` typealiases can only carry types representable +/// in Objective-C, and a pointer to a non-`@objc` Swift struct +/// fails that check. The Rust side ships a `*const PersistKeyArgs` +/// regardless; the Swift trampoline unwraps via +/// `assumingMemoryBound(to: PersistKeyArgs.self)`. Same ABI shape +/// as the Rust `extern "C" fn(_, *const PersistKeyArgs) -> u8` +/// declaration. +typealias PersistKeyCallback = @convention(c) ( + _ ctx: UnsafeRawPointer?, + _ args: UnsafeRawPointer? +) -> UInt8 + +/// Persister-callback success/failure tags. Mirror the +/// `PERSIST_KEY_SUCCESS` / `PERSIST_KEY_FAILURE` constants in +/// `derive_and_persist_callbacks.rs`. Trampoline implementations +/// return one of these to keep the wire shape consistent. +let PERSIST_KEY_SUCCESS: UInt8 = 1 +let PERSIST_KEY_FAILURE: UInt8 = 0 + +/// Generic Rust-callable destructor for any Swift-owned `ctx` +/// pointer (typically `Unmanaged.passRetained(self).toOpaque()`). +typealias DeriveAndPersistCtxDestroy = @convention(c) ( + _ ctx: UnsafeMutableRawPointer? +) -> Void + +@_silgen_name("dash_sdk_mnemonic_resolver_create") +func dash_sdk_mnemonic_resolver_create( + _ ctx: UnsafeMutableRawPointer?, + _ resolve_callback: MnemonicResolveCallback, + _ destroy_callback: DeriveAndPersistCtxDestroy +) -> UnsafeMutablePointer? + +@_silgen_name("dash_sdk_mnemonic_resolver_destroy") +func dash_sdk_mnemonic_resolver_destroy( + _ handle: UnsafeMutablePointer? +) + +@_silgen_name("dash_sdk_identity_key_persister_create") +func dash_sdk_identity_key_persister_create( + _ ctx: UnsafeMutableRawPointer?, + _ persist_callback: PersistKeyCallback, + _ destroy_callback: DeriveAndPersistCtxDestroy +) -> UnsafeMutablePointer? + +@_silgen_name("dash_sdk_identity_key_persister_destroy") +func dash_sdk_identity_key_persister_destroy( + _ handle: UnsafeMutablePointer? +) + +/// Single-FFI identity-key derivation + persistence pipeline. +/// +/// Companion to the lower-level +/// `dash_sdk_derive_identity_keys_from_mnemonic` (which leaves +/// orchestration to the caller). This entry point owns the +/// per-key loop on the Rust side and calls back into Swift only +/// for the iOS-only Keychain primitives. +/// +/// On success `out_pubkeys` is populated with the derived +/// pubkeys (and their paths) for the caller to build +/// `IdentityPubkey` rows for the subsequent registration call; +/// the 32-byte ECDSA private scalars are NOT included — they +/// were already handed to the persister callback per iteration. +/// Release `out_pubkeys` with +/// `dash_sdk_derive_identity_keys_from_mnemonic_free` (same +/// memory layout, intentionally shares the free function). +@_silgen_name("dash_sdk_derive_and_persist_identity_keys") +func dash_sdk_derive_and_persist_identity_keys( + _ network: DashSDKNetwork, + _ wallet_id_bytes: UnsafePointer?, + _ identity_index: UInt32, + _ key_count: UInt32, + _ mnemonic_resolver_handle: UnsafeMutablePointer?, + _ persister_handle: UnsafeMutablePointer?, + _ out_pubkeys: UnsafeMutablePointer, + _ out_error: UnsafeMutablePointer +) -> PlatformWalletFFIResult + +// MARK: - Resolver-driven one-shot sign +// +// Sibling of the lower-level `dash_sdk_sign_with_mnemonic_and_path` +// in rs-sdk-ffi. Routes the mnemonic fetch through a Swift-owned +// `MnemonicResolverHandle` instead of taking it as a raw C-string, +// closing the same swift-sdk/CLAUDE.md "no mnemonic round-tripping" +// rule for the platform-address signing path. + +/// Mirrors the Rust `SIGN_WITH_RESOLVER_*` byte tags. Returned via +/// the `out_error` byte parameter on a non-zero rc. +enum SignWithMnemonicResolverError: UInt8 { + case ok = 0 + case nullPointer = 1 + case invalidUtf8 = 2 + case invalidMnemonic = 3 + case invalidPath = 4 + case derivationFailed = 5 + case signFailed = 6 + case bufferTooSmall = 7 + case unsupportedKeyType = 8 + case resolverNotFound = 9 + case resolverFailed = 10 +} + +@_silgen_name("dash_sdk_sign_with_mnemonic_resolver_and_path") +func dash_sdk_sign_with_mnemonic_resolver_and_path( + _ mnemonic_resolver_handle: UnsafeMutablePointer?, + _ wallet_id_bytes: UnsafePointer?, + _ derivation_path_cstr: UnsafePointer?, + _ data: UnsafePointer?, + _ data_len: Int, + _ key_type: UInt8, + _ network: DashSDKNetwork, + _ out_signature: UnsafeMutablePointer?, + _ out_signature_capacity: Int, + _ out_signature_len: UnsafeMutablePointer?, + _ out_error: UnsafeMutablePointer? +) -> Int32 diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift index 0fb978b902c..eb4aa51557e 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swift @@ -429,39 +429,21 @@ struct CreateIdentityView: View { // buffers immediately. The mnemonic stays in Keychain; the // derivation path lives on `PersistentPlatformAddress`. let signer = KeychainSigner(modelContainer: modelContext.container) - let walletIdHex = walletId.map { String(format: "%02x", $0) }.joined() let identityPubkeys: [ManagedPlatformWallet.IdentityPubkey] do { - // `prePersistIdentityKeysForRegistration` derives every key - // via the watch-only-safe mnemonic FFI and writes the - // private key bytes into Keychain. The returned previews - // already carry the public-key bytes — capture them here - // and thread them through to - // `registerIdentityFromAddresses` so the Rust side does - // NOT re-derive (which would fail for watch-only wallets - // restored from SwiftData state). - // - // Convention for identity-registration auth keys (matches - // the prior Rust-side derivation that lived inside - // `platform_wallet_register_identity_with_signer`): - // - keyId 0 → MASTER, AUTHENTICATION, ECDSA_SECP256K1, !readOnly - // - keyId > 0 → HIGH, AUTHENTICATION, ECDSA_SECP256K1, !readOnly - let previews = try managedWallet.prePersistIdentityKeysForRegistration( + // Single-FFI derive + persist. The Rust side owns the + // per-key MASTER-vs-HIGH policy and ships back the + // ready-to-use `IdentityPubkey` rows; the Swift caller + // does not recreate the policy. See + // swift-sdk/CLAUDE.md "no mnemonic round-tripping" — + // both the mnemonic fetch and the per-key Keychain + // write are now Rust → Swift callbacks orchestrated by + // the Rust loop, not Swift-side glue. + identityPubkeys = try managedWallet.prePersistIdentityKeysForRegistration( identityIndex: identityIndex, keyCount: Self.defaultKeyCount, - network: platformState.currentNetwork.sdkNetwork, - walletIdHex: walletIdHex + network: platformState.currentNetwork.sdkNetwork ) - identityPubkeys = previews.enumerated().map { (i, preview) in - ManagedPlatformWallet.IdentityPubkey( - keyId: UInt32(i), - keyType: .ecdsaSecp256k1, - purpose: .authentication, - securityLevel: i == 0 ? .master : .high, - pubkeyBytes: preview.publicKeyData, - readOnly: false - ) - } } catch { submitError = .init( message: "Could not pre-derive identity keys: \(error.localizedDescription)" From 2f6c4654d09257914709ba9d8ba9a8617b798b0e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 26 Apr 2026 20:37:08 +0800 Subject: [PATCH 2/5] fix(platform): address PR #3542 review (leak, validation, ABI guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight review comments on PR #3542; this commit addresses all of them. Memory leak (BLOCKING — thepastaclaw): - The `Unmanaged.passRetained(self).toOpaque()` pattern in init created a circular ownership: Rust held a +1 retain on `self`, the destroy trampoline only released it when the destroy FFI fired, and the destroy FFI was only invoked from `deinit` — which ARC could never enter while the +1 retain was alive. Result: every MnemonicResolver, IdentityKeyPersister, AND KeychainSigner instance leaked forever. The leak in KeychainSigner was pre-existing; the new wrappers inherited the same shape and added two more leaks per registration. - Switched all three classes to `passUnretained`. ARC alone now controls when these objects deallocate; `deinit` calls the destroy FFI which frees the Rust handle/vtable allocations. The destroy trampolines are no-ops (no extra retain to balance). Caller must keep the Swift instance alive for the duration of any FFI call that captured the handle — the natural pattern (`let resolver = MnemonicResolver(...)` followed by a synchronous FFI call, or `let signer = KeychainSigner(...)` followed by `try await ...registerIdentity(signer:)`) does this by construction. Validation (CodeRabbit Major): - IdentityKeyPersister now validates the derived private key against the public key via `KeyValidation.validatePrivateKeyForPublicKey` before writing to Keychain. Per swift-sdk/CLAUDE.md "Always validate private keys match their public keys". Catches any future ABI / payload corruption at the persist boundary instead of letting a mismatched row land in the Keychain and surface as an opaque "signature invalid" error months later. - KeychainSigner.signPlatformAddressOnDemand now guards `ctx.walletId.count == 32` before forwarding the pointer to `dash_sdk_sign_with_mnemonic_resolver_and_path`. A truncated / corrupt PersistentPlatformAddress.walletId would otherwise cause Rust to read past the buffer; now it surfaces as a clean `walletIdInvalidLength(actual:expected:)` typed error. Error surfacing (CodeRabbit + thepastaclaw): - `prePersistIdentityKeysForRegistration` no longer `precondition`s on FFI count drift — converts to `PlatformWalletError.walletOperation` with the count mismatch in the message. Also rejects partial output (`out.count != Int(keyCount)`) where it previously silently proceeded. - The `??` defaults that masked unknown DPP enum bytes are gone. Unknown `keyType` / `purpose` / `securityLevel` discriminants now throw `PlatformWalletError.walletOperation` with the offending bytes in the message. Defaulting silently would have changed key semantics if Rust's enum discriminants ever drifted. - MnemonicResolver trampoline now distinguishes `WalletStorageError.mnemonicNotFound` (→ resolver `notFound`, surfaces as the `mnemonicMissing(walletIdHex:)` user-facing error) from any other error (→ resolver `other`, surfaces as the generic `signWithMnemonicFailed` so locked-Keychain or biometric-auth failures aren't reported as "no mnemonic stored". ABI drift guard (CodeRabbit Refactor): - `PersistKeyArgs` gained a 72-byte size assertion on both sides, mirroring the existing `IdentityKeyEntryFFI` pattern. Rust uses a `const _: [u8; 72]` size + alignment check; Swift calls `assertPersistKeyArgsLayout()` once at IdentityKeyPersister init. Drift between the two `#[repr(C)]` mirrors now fails fast at build / construct time, not via EXC_BAD_ACCESS in `assumingMemoryBound`. Test race (CodeRabbit Minor): - The shared `static CAPTURED: Mutex>` in `identity_derive_and_persist::tests` was flaky under cargo's default parallel-tests mode: tests cleared and asserted on the same buffer concurrently. Replaced with per-test `Box` passed via the persister `ctx` pointer (the FFI was designed for exactly this). The static is gone. Verification: - cargo clippy -p platform-wallet-ffi -p platform-wallet -p rs-sdk-ffi --all-targets -- -D warnings: clean. - cargo test -p platform-wallet-ffi --lib: 58/58 pass. - cargo machete: clean across all three crates. - ./build_ios.sh --target sim: SwiftExampleApp builds and links cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/derive_and_persist_callbacks.rs | 29 +++++ .../src/identity_derive_and_persist.rs | 88 ++++++++++--- .../SwiftDashSDK/FFI/KeychainSigner.swift | 83 +++++++++--- .../FFI/MnemonicResolverAndPersister.swift | 120 ++++++++++++++---- .../ManagedPlatformWallet.swift | 37 +++++- .../PlatformWallet/PlatformWalletFFI.swift | 24 ++++ 6 files changed, 305 insertions(+), 76 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs b/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs index b7b98d9da91..18ebab97eab 100644 --- a/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs +++ b/packages/rs-platform-wallet-ffi/src/derive_and_persist_callbacks.rs @@ -255,6 +255,35 @@ pub struct PersistKeyArgs { pub security_level: u8, } +// Compile-time layout assertions. Mirrors the runtime +// `assertPersistKeyArgsLayout` check on the Swift side. If any +// field is added / reordered / resized below, BOTH sides need +// to be updated; the constants here turn drift into a build +// failure rather than an EXC_BAD_ACCESS at trampoline time. +// +// Layout on 64-bit ABI: +// +// | offset | field | size | +// |--------|------------------------|------| +// | 0 | wallet_id_bytes | 8 | +// | 8 | identity_index | 4 | +// | 12 | key_id | 4 | +// | 16 | key_index | 4 | +// | 20 | (padding) | 4 | +// | 24 | derivation_path_cstr | 8 | +// | 32 | public_key_bytes | 8 | +// | 40 | public_key_len | 8 | +// | 48 | public_key_hash_bytes | 8 | +// | 56 | private_key_bytes | 8 | +// | 64 | key_type | 1 | +// | 65 | purpose | 1 | +// | 66 | security_level | 1 | +// | 67 | (padding) | 5 | +// +// Total = 72 bytes, alignment = 8 (pointer / usize). +const _: [u8; 72] = [0u8; std::mem::size_of::()]; +const _: [u8; 8] = [0u8; std::mem::align_of::()]; + /// Function pointer type for the per-key persist callback. /// /// Returns a non-zero `u8` on a successful persist, `0` on diff --git a/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs b/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs index cbe4631d7c4..13d339ae6dc 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_derive_and_persist.rs @@ -473,11 +473,6 @@ mod tests { const ENGLISH_PHRASE: &str = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; - /// Capture for what the persister callback received. Behind a - /// Mutex because the FFI callback is `extern "C"` and can't - /// hold a `&mut`. - static CAPTURED: Mutex> = Mutex::new(Vec::new()); - #[derive(Debug, Clone)] struct CapturedPersist { identity_index: u32, @@ -493,6 +488,15 @@ mod tests { wallet_id: [u8; 32], } + /// Per-test capture buffer the persister callback writes + /// into. Each test allocates its own `Box` and + /// passes the raw pointer as the persister `ctx`, so tests + /// running in parallel cannot stomp each other (cargo runs + /// tests in the same module concurrently by default). + struct CaptureCtx { + rows: Mutex>, + } + unsafe extern "C" fn capturing_resolve( _ctx: *const c_void, _wallet_id_bytes: *const u8, @@ -512,7 +516,14 @@ mod tests { unsafe extern "C" fn noop_destroy(_ctx: *mut c_void) {} - unsafe extern "C" fn capturing_persist(_ctx: *const c_void, args: *const PersistKeyArgs) -> u8 { + /// Persister that writes to the per-test [`CaptureCtx`]. The + /// `ctx` pointer is the `Box::into_raw` of a `CaptureCtx` + /// allocated by the test. + unsafe extern "C" fn capturing_persist(ctx: *const c_void, args: *const PersistKeyArgs) -> u8 { + if ctx.is_null() { + return crate::derive_and_persist_callbacks::PERSIST_KEY_FAILURE; + } + let capture = &*(ctx as *const CaptureCtx); let a = &*args; let path = CStr::from_ptr(a.derivation_path_cstr) .to_string_lossy() @@ -522,7 +533,7 @@ mod tests { let private_key = std::slice::from_raw_parts(a.private_key_bytes, 32).to_vec(); let mut wallet_id = [0u8; 32]; std::ptr::copy_nonoverlapping(a.wallet_id_bytes, wallet_id.as_mut_ptr(), 32); - CAPTURED.lock().unwrap().push(CapturedPersist { + capture.rows.lock().unwrap().push(CapturedPersist { identity_index: a.identity_index, key_id: a.key_id, key_index: a.key_index, @@ -542,9 +553,38 @@ mod tests { crate::derive_and_persist_callbacks::PERSIST_KEY_FAILURE } - fn make_handles( - persist: crate::derive_and_persist_callbacks::PersistKeyCallback, - ) -> (*mut MnemonicResolverHandle, *mut IdentityKeyPersisterHandle) { + /// Allocate a fresh capture context + paired handles. + /// Returns `(resolver, persister, capture_box_ptr)` — the + /// capture pointer must be freed (via `Box::from_raw`) after + /// the test's assertions have run. + fn make_capturing_handles() -> ( + *mut MnemonicResolverHandle, + *mut IdentityKeyPersisterHandle, + *mut CaptureCtx, + ) { + let capture = Box::into_raw(Box::new(CaptureCtx { + rows: Mutex::new(Vec::new()), + })); + unsafe { + ( + dash_sdk_mnemonic_resolver_create( + std::ptr::null_mut(), + capturing_resolve, + noop_destroy, + ), + dash_sdk_identity_key_persister_create( + capture as *mut c_void, + capturing_persist, + noop_destroy, + ), + capture, + ) + } + } + + /// Allocate handles where the persister always returns + /// failure. No capture buffer is needed. + fn make_failing_handles() -> (*mut MnemonicResolverHandle, *mut IdentityKeyPersisterHandle) { unsafe { ( dash_sdk_mnemonic_resolver_create( @@ -552,15 +592,18 @@ mod tests { capturing_resolve, noop_destroy, ), - dash_sdk_identity_key_persister_create(std::ptr::null_mut(), persist, noop_destroy), + dash_sdk_identity_key_persister_create( + std::ptr::null_mut(), + failing_persist, + noop_destroy, + ), ) } } #[test] fn happy_path_persists_three_keys_and_returns_pubkeys() { - CAPTURED.lock().unwrap().clear(); - let (resolver, persister) = make_handles(capturing_persist); + let (resolver, persister, capture) = make_capturing_handles(); let wallet_id = [42u8; 32]; let mut out = IdentityRegistrationKeyDerivationsFFI { items: std::ptr::null_mut(), @@ -582,7 +625,10 @@ mod tests { assert_eq!(rc, PlatformWalletFFIResult::Success); assert_eq!(out.count, 3); - let captured = CAPTURED.lock().unwrap().clone(); + // SAFETY: `capture` is alive (we're about to free it + // below) and the persister can no longer fire — the FFI + // call has already returned synchronously. + let captured = unsafe { (*capture).rows.lock().unwrap().clone() }; assert_eq!(captured.len(), 3); // key_id, key_index sequence @@ -616,13 +662,14 @@ mod tests { crate::dash_sdk_derive_identity_keys_from_mnemonic_free(&mut out); dash_sdk_mnemonic_resolver_destroy(resolver); dash_sdk_identity_key_persister_destroy(persister); + // Reclaim the per-test capture box. + let _ = Box::from_raw(capture); } } #[test] fn returning_false_from_persister_aborts_with_wallet_op_error() { - CAPTURED.lock().unwrap().clear(); - let (resolver, persister) = make_handles(failing_persist); + let (resolver, persister) = make_failing_handles(); let wallet_id = [1u8; 32]; let mut out = IdentityRegistrationKeyDerivationsFFI { items: std::ptr::null_mut(), @@ -652,7 +699,7 @@ mod tests { #[test] fn rejects_null_inputs() { - let (resolver, persister) = make_handles(capturing_persist); + let (resolver, persister, capture) = make_capturing_handles(); let wallet_id = [0u8; 32]; let mut err = PlatformWalletFFIError::success(); // Null out_pubkeys. @@ -691,13 +738,13 @@ mod tests { unsafe { dash_sdk_mnemonic_resolver_destroy(resolver); dash_sdk_identity_key_persister_destroy(persister); + let _ = Box::from_raw(capture); } } #[test] fn key_count_zero_is_success_no_calls() { - CAPTURED.lock().unwrap().clear(); - let (resolver, persister) = make_handles(capturing_persist); + let (resolver, persister, capture) = make_capturing_handles(); let wallet_id = [0u8; 32]; let mut out = IdentityRegistrationKeyDerivationsFFI { items: std::ptr::null_mut(), @@ -718,10 +765,11 @@ mod tests { }; assert_eq!(rc, PlatformWalletFFIResult::Success); assert_eq!(out.count, 0); - assert_eq!(CAPTURED.lock().unwrap().len(), 0); + assert_eq!(unsafe { (*capture).rows.lock().unwrap().len() }, 0); unsafe { dash_sdk_mnemonic_resolver_destroy(resolver); dash_sdk_identity_key_persister_destroy(persister); + let _ = Box::from_raw(capture); } } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index a70dd7c2cf0..b68f59aa218 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -56,13 +56,22 @@ import SwiftData /// # Lifetime /// /// `init` allocates a `*mut SignerHandle` via -/// `dash_sdk_signer_create_with_ctx`. The Rust side captures the -/// vtable + the opaque `ctx` (which is -/// `Unmanaged.passRetained(self).toOpaque()` — a +1 retain on `self`). -/// `deinit` calls `dash_sdk_signer_destroy`, which fires our -/// destructor trampoline, which drops the +1 retain. The handle is -/// safe to pass to FFI calls for the entire lifetime of the -/// `KeychainSigner` instance. +/// `dash_sdk_signer_create_with_ctx` and registers `self` as the +/// opaque ctx via `Unmanaged.passUnretained(self).toOpaque()` — +/// a non-owning pointer. ARC alone controls when this object +/// deallocates; the caller must keep the `KeychainSigner` +/// instance alive for the duration of any FFI call that +/// captured `handle`. `deinit` calls +/// `dash_sdk_signer_destroy` to free the Rust-side handle/vtable +/// allocations; the destroy trampoline is a no-op (no extra +/// retain to balance). +/// +/// Earlier revisions of this file used `passRetained`, which +/// created a circular ownership: Rust held a +1 retain on `self`, +/// the destroy trampoline only released it when the destroy FFI +/// fired, and the destroy FFI was only invoked from `deinit` — +/// which ARC could never enter while the +1 retain was alive. +/// Result: every `KeychainSigner` instance leaked forever. public final class KeychainSigner: Signer, @unchecked Sendable { // MARK: Public surface @@ -94,6 +103,12 @@ public final class KeychainSigner: Signer, @unchecked Sendable { /// `key_type == 0xFF` branch: the wallet-mnemonic Keychain /// item is missing for the wallet that owns this address. case mnemonicMissing(walletIdHex: String) + /// `key_type == 0xFF` branch: the resolved + /// `PersistentPlatformAddress.walletId` was not the + /// expected 32-byte length. Indicates a corrupt persister + /// write — every row should carry a wallet id matching + /// the wallet's `walletId` field exactly. + case walletIdInvalidLength(actual: Int, expected: Int) /// `dash_sdk_sign_with_mnemonic_and_path` returned a /// non-zero error tag. The tag is the byte written to /// `out_error` (see `SIGN_WITH_MNEMONIC_ERR_*` constants in @@ -118,6 +133,8 @@ public final class KeychainSigner: Signer, @unchecked Sendable { return "PersistentPlatformAddress row for \(hashHex) has no derivation path." case .mnemonicMissing(let walletIdHex): return "No Keychain mnemonic stored for wallet \(walletIdHex)." + case .walletIdInvalidLength(let actual, let expected): + return "PersistentPlatformAddress.walletId is \(actual) bytes; expected \(expected)." case .signWithMnemonicFailed(let tag): return "dash_sdk_sign_with_mnemonic_and_path failed with error tag \(tag)." } @@ -175,11 +192,16 @@ public final class KeychainSigner: Signer, @unchecked Sendable { // reference. Used by the platform-address signing branch. self.mnemonicResolver = MnemonicResolver() - // Hand Rust an opaque `+1`-retained pointer to self. The - // matching release happens in `keychainSignerDestroyTrampoline` - // when the FFI handle is destroyed. Until then `self` cannot - // be deallocated even if Swift drops every other reference. - let ctx = Unmanaged.passRetained(self).toOpaque() + // Hand Rust an opaque NON-owning pointer to self. The + // Swift owner is responsible for keeping `self` alive + // for the duration of any in-flight FFI call that + // captured the handle (the natural pattern: `let signer + // = KeychainSigner(...)` followed by an `await + // ...registerIdentity(...signer:)` keeps the local + // `signer` alive until the await completes). See the + // type-level "Lifetime" note for why `passRetained` + // would leak. + let ctx = Unmanaged.passUnretained(self).toOpaque() let handlePtr = dash_sdk_signer_create_with_ctx( ctx, @@ -195,10 +217,12 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } deinit { - // `dash_sdk_signer_destroy` drops the box, which fires the - // vtable destructor trampoline. The trampoline performs the - // matching `Unmanaged.fromOpaque(...).release()`, balancing - // the `passRetained` from `init`. + // `dash_sdk_signer_destroy` drops the Rust handle box + + // vtable allocation. The destroy trampoline is a no-op + // (init used `passUnretained`, so there's nothing to + // release). Caller must ensure no in-flight FFI calls + // still reference `handlePtr` at the moment ARC fires + // this `deinit`. if let handlePtr { dash_sdk_signer_destroy(handlePtr) } @@ -416,6 +440,22 @@ public final class KeychainSigner: Signer, @unchecked Sendable { return .failure(.platformAddressNotFound(addressHashHex: hashHex)) } + // Validate the wallet-id length BEFORE shipping the + // pointer across the FFI. Rust's + // `dash_sdk_sign_with_mnemonic_resolver_and_path` reads + // exactly 32 bytes from `wallet_id_bytes`; a truncated / + // corrupt `PersistentPlatformAddress.walletId` row would + // cause a read past the buffer rather than a clean + // failure. Surface the precise reason here so the + // explorer / log makes the corruption obvious. + let expectedWalletIdLen = 32 + guard ctx.walletId.count == expectedWalletIdLen else { + return .failure(.walletIdInvalidLength( + actual: ctx.walletId.count, + expected: expectedWalletIdLen + )) + } + // Buffer sized generously (128B) — ECDSA compact-recoverable // signatures are 65 bytes; the cap leaves room for future // signature-shape additions without an ABI change. Defer- @@ -683,8 +723,9 @@ private func keychainSignerCanSignTrampoline( } /// Vtable destructor — invoked exactly once when the FFI handle is -/// destroyed. Drops the `+1` retain captured by `init`. -private func keychainSignerDestroyTrampoline(ctx: UnsafeMutableRawPointer?) { - guard let ctx else { return } - Unmanaged.fromOpaque(ctx).release() -} +/// destroyed. No-op now that init uses `passUnretained`: there is +/// no extra retain to balance, and ARC has already deallocated +/// (or is in the process of deallocating) `self` by the time +/// `dash_sdk_signer_destroy` runs from `deinit`. Kept around so +/// the Rust vtable's `destroy` slot is always non-null. +private func keychainSignerDestroyTrampoline(_: UnsafeMutableRawPointer?) {} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift index 9294775655d..8285c3cc2f2 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift @@ -6,26 +6,47 @@ import Foundation /// Swift bridge backing the Rust-side `MnemonicResolverHandle`. /// /// The Rust derivation loop in -/// `dash_sdk_derive_and_persist_identity_keys` calls back into -/// Swift via this resolver to fetch the BIP-39 mnemonic for the -/// wallet whose identity keys it's deriving. The mnemonic is +/// `dash_sdk_derive_and_persist_identity_keys` (and the +/// platform-address signing path in +/// `dash_sdk_sign_with_mnemonic_resolver_and_path`) calls back +/// into Swift via this resolver to fetch the BIP-39 mnemonic for +/// the wallet whose identity keys it's deriving. The mnemonic is /// copied directly into a Rust-owned `Zeroizing` stack buffer; it /// never round-trips back to Swift after this single read, and /// the Swift `String` from `WalletStorage.retrieveMnemonic` falls /// out of scope at the end of the trampoline. /// -/// # Lifetime +/// # Lifetime contract /// -/// Init allocates a `MnemonicResolverHandle*` via -/// `dash_sdk_mnemonic_resolver_create`, handing Rust an -/// `Unmanaged.passRetained(self)` pointer. `deinit` calls -/// `dash_sdk_mnemonic_resolver_destroy`, which fires the -/// destructor trampoline that releases the +1 retain. The handle -/// is safe to pass to FFI calls for the entire lifetime of this -/// instance. +/// Init does NOT retain `self` through the Rust handle — +/// `Unmanaged.passUnretained` is used so that ARC alone controls +/// when this object deallocates. The Swift owner is responsible +/// for keeping the instance alive for the duration of any FFI +/// call that captured `handle`. In practice that's automatic for +/// the synchronous `dash_sdk_derive_and_persist_identity_keys` / +/// `dash_sdk_sign_with_mnemonic_resolver_and_path` paths, where +/// a local `let resolver = MnemonicResolver(...)` variable +/// outlives the call by construction. +/// +/// `deinit` calls `dash_sdk_mnemonic_resolver_destroy` to free +/// the Rust-side handle/vtable allocations. +/// +/// # Why not `passRetained`? +/// +/// Using `passRetained` here would create a circular ownership: +/// Rust holds a +1 retain on `self`, the destroy trampoline only +/// releases that retain when the destroy FFI is invoked, and the +/// destroy FFI is only invoked from `deinit`. ARC won't run +/// `deinit` while the +1 retain is alive → the handle leaks +/// every instance forever. Per-call usage in +/// `prePersistIdentityKeysForRegistration` would leak two of +/// these per registration. public final class MnemonicResolver: @unchecked Sendable { /// Owned for the lifetime of this object. Pass this to - /// `dash_sdk_derive_and_persist_identity_keys`. + /// `dash_sdk_derive_and_persist_identity_keys` (or the + /// resolver-based sign FFI). Caller must keep `self` alive + /// for the duration of any FFI call that captured the + /// handle — see the type-level "Lifetime contract" note. public var handle: UnsafeMutablePointer? { handlePtr } @@ -39,7 +60,7 @@ public final class MnemonicResolver: @unchecked Sendable { public init(storage: WalletStorage = WalletStorage()) { self.storage = storage - let ctx = Unmanaged.passRetained(self).toOpaque() + let ctx = Unmanaged.passUnretained(self).toOpaque() self.handlePtr = dash_sdk_mnemonic_resolver_create( ctx, mnemonicResolverResolveTrampoline, @@ -67,8 +88,21 @@ public final class MnemonicResolver: @unchecked Sendable { let mnemonic: String do { mnemonic = try storage.retrieveMnemonic(for: walletId) - } catch { + } catch WalletStorageError.mnemonicNotFound { + // Distinct "this wallet has no stored mnemonic" case + // — Rust surfaces this as the recoverable + // `mnemonicMissing(walletIdHex:)` error in the Swift + // signing path, which UI uses to direct the user + // toward "import this wallet's mnemonic". return .notFound + } catch { + // Anything else (Keychain access denied, biometric + // auth failed, OSStatus failure...) is a real + // operational error. Map to `.other` so the + // distinction survives across the FFI boundary — + // collapsing into `.notFound` would point the user + // toward the wrong remediation. + return .other } // `withCString` materializes a null-terminated UTF-8 byte @@ -94,11 +128,6 @@ public final class MnemonicResolver: @unchecked Sendable { } // MARK: - MnemonicResolver C trampolines -// -// These run on whatever thread the Rust derivation loop is -// executing on (typically a background queue from the iOS -// caller). `WalletStorage.retrieveMnemonic` is thread-safe; no -// further synchronization is required. private func mnemonicResolverResolveTrampoline( ctx: UnsafeRawPointer?, @@ -121,10 +150,11 @@ private func mnemonicResolverResolveTrampoline( return result.rawValue } -private func mnemonicResolverDestroyTrampoline(ctx: UnsafeMutableRawPointer?) { - guard let ctx else { return } - Unmanaged.fromOpaque(ctx).release() -} +/// No-op destructor. The Swift `self` is held by ARC alone — the +/// `passUnretained` ctx pointer carries no refcount to balance. +/// Keeping the trampoline so the Rust vtable's `destroy` slot is +/// always non-null (simplifies the Rust safety contract). +private func mnemonicResolverDestroyTrampoline(_: UnsafeMutableRawPointer?) {} // MARK: - IdentityKeyPersister @@ -139,6 +169,9 @@ private func mnemonicResolverDestroyTrampoline(ctx: UnsafeMutableRawPointer?) { /// whatever bytes it's told into Keychain. This is the inverse of /// the prior shape, where Swift hardcoded `keyId 0 -> MASTER, /// else HIGH` plus the DPP discriminant bytes. +/// +/// Same `passUnretained` lifetime contract as +/// [`MnemonicResolver`] — see that type's docs. public final class IdentityKeyPersister: @unchecked Sendable { /// Per-key metadata captured as a side effect of each persist /// callback firing during a `dash_sdk_derive_and_persist_identity_keys` @@ -181,9 +214,14 @@ public final class IdentityKeyPersister: @unchecked Sendable { private var handlePtr: UnsafeMutablePointer? public init(keychain: KeychainManager = .shared) { + // Layout sanity check before the trampoline starts firing. + // Catches Rust `#[repr(C)]` drift at construction rather + // than letting `assumingMemoryBound` blow up mid-derive. + assertPersistKeyArgsLayout() + self.keychain = keychain - let ctx = Unmanaged.passRetained(self).toOpaque() + let ctx = Unmanaged.passUnretained(self).toOpaque() self.handlePtr = dash_sdk_identity_key_persister_create( ctx, identityKeyPersisterPersistTrampoline, @@ -227,6 +265,34 @@ public final class IdentityKeyPersister: @unchecked Sendable { .joined() let privateKeyData = Data(bytes: privKeyPtr, count: 32) + // Per swift-sdk/CLAUDE.md "Always validate private keys + // match their public keys using + // KeyValidation.validatePrivateKeyForPublicKey". Catches + // any future ABI / payload corruption (e.g. struct + // padding drift, byte-order mistake, wrong-key-shipped + // bug) at the persist boundary instead of letting a + // mismatched row land in the keychain and surface as an + // opaque "signature invalid" error months later. + guard let resolvedKeyType = KeyType(rawValue: a.key_type) else { + return false + } + let privateKeyHex = privateKeyData + .map { String(format: "%02x", $0) } + .joined() + // `isTestnet` only affects WIF address derivation inside + // the validator, not the underlying ECDSA scalar→pubkey + // correspondence we actually care about — pass `true` as + // a stable default. + let validates = KeyValidation.validatePrivateKeyForPublicKey( + privateKeyHex: privateKeyHex, + publicKeyHex: publicKeyHex, + keyType: resolvedKeyType, + isTestnet: true + ) + guard validates else { + return false + } + // Identity-id is unknown pre-registration — Rust will // recompute it from the input addresses at submit time. // Use the marker `pending` so the keychain explorer makes @@ -282,7 +348,5 @@ private func identityKeyPersisterPersistTrampoline( return persister.persist(args: typedArgs) ? PERSIST_KEY_SUCCESS : PERSIST_KEY_FAILURE } -private func identityKeyPersisterDestroyTrampoline(ctx: UnsafeMutableRawPointer?) { - guard let ctx else { return } - Unmanaged.fromOpaque(ctx).release() -} +/// No-op destructor — see `mnemonicResolverDestroyTrampoline`. +private func identityKeyPersisterDestroyTrampoline(_: UnsafeMutableRawPointer?) {} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index c950e515026..2fb4a39e65e 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -750,11 +750,23 @@ extension ManagedPlatformWallet { // — both indexed in derivation order. Rust decided // (keyType, purpose, securityLevel); Swift just maps each // discriminant byte back into its strongly-typed enum. + // + // Both an FFI/Swift count drift AND any unknown enum + // discriminant byte are surfaced as a recoverable + // `PlatformWalletError.walletOperation` rather than a + // process abort or a silently-defaulted enum value. + // Defaulting `securityLevel` etc. to a known constant + // (the prior shape) would silently change key semantics + // if Rust's enum discriminants ever drifted; better to + // fail loudly so the caller learns about the ABI break + // immediately. let captured = persister.persistedKeys - precondition( - captured.count == out.count, - "persister callback fired \(captured.count) times but FFI returned \(out.count) pubkey rows" - ) + guard captured.count == out.count, out.count == Int(keyCount) else { + throw PlatformWalletError.walletOperation( + "derive_and_persist_identity_keys returned \(out.count) pubkeys for " + + "\(keyCount) requested keys; persister captured \(captured.count)" + ) + } var pubkeys: [IdentityPubkey] = [] pubkeys.reserveCapacity(out.count) @@ -767,12 +779,23 @@ extension ManagedPlatformWallet { pubData = Data() } let meta = captured[i] + guard + let keyType = KeyType(rawValue: meta.keyType), + let purpose = KeyPurpose(rawValue: meta.purpose), + let securityLevel = SecurityLevel(rawValue: meta.securityLevel) + else { + throw PlatformWalletError.walletOperation( + "derive_and_persist_identity_keys returned unknown enum bytes " + + "(keyType=\(meta.keyType), purpose=\(meta.purpose), " + + "securityLevel=\(meta.securityLevel)) for keyId=\(meta.keyId)" + ) + } pubkeys.append( IdentityPubkey( keyId: meta.keyId, - keyType: KeyType(rawValue: meta.keyType) ?? .ecdsaSecp256k1, - purpose: KeyPurpose(rawValue: meta.purpose) ?? .authentication, - securityLevel: SecurityLevel(rawValue: meta.securityLevel) ?? .high, + keyType: keyType, + purpose: purpose, + securityLevel: securityLevel, pubkeyBytes: pubData, readOnly: false ) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift index 71354869c1a..ae00026dafc 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletFFI.swift @@ -1397,6 +1397,30 @@ struct PersistKeyArgs { var security_level: UInt8 } +/// Expected size of `PersistKeyArgs` as laid out by Rust's +/// `#[repr(C)]` on 64-bit targets. Mirrors the compile-time +/// assertion in `rs-platform-wallet-ffi/src/ +/// derive_and_persist_callbacks.rs`. Tested at trampoline entry +/// via `assertPersistKeyArgsLayout()`. +let EXPECTED_PERSIST_KEY_ARGS_SIZE: Int = 72 + +/// Verify the Swift `PersistKeyArgs` mirror lays out to the same +/// 72-byte shape Rust's `#[repr(C)]` produces. Called once per +/// process from the persister-callback hot path so a drift +/// between the two sides surfaces as a clean assertion failure +/// rather than an EXC_BAD_ACCESS in `assumingMemoryBound`. +func assertPersistKeyArgsLayout() { + let actual = MemoryLayout.size + let actualStride = MemoryLayout.stride + precondition( + actual == EXPECTED_PERSIST_KEY_ARGS_SIZE + && actualStride == EXPECTED_PERSIST_KEY_ARGS_SIZE, + "PersistKeyArgs layout mismatch: size=\(actual) stride=\(actualStride), " + + "expected \(EXPECTED_PERSIST_KEY_ARGS_SIZE). Rust-side " + + "#[repr(C)] and Swift-side struct have diverged; fix one side." + ) +} + /// Function pointer type for the per-key persist callback. /// Returns [`PERSIST_KEY_SUCCESS`] on a successful Keychain write, /// [`PERSIST_KEY_FAILURE`] to abort the rest of the Rust derivation From e9d17d382375eb0e883a8d715ba246b3bb03e95d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 26 Apr 2026 21:06:50 +0800 Subject: [PATCH 3/5] fix(swift-sdk): distinguish address-resolve cases + reject empty pubkey rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-up review comments from CodeRabbit on PR #3542 after the prior fix push. KeychainSigner.resolvePlatformAddressContext (CodeRabbit Minor): - Return type changed from `PlatformAddressContext?` to a richer `PlatformAddressResolution` enum with `.found / .noMatch / .rowMatchedButPathEmpty`. The prior shape collapsed both failure modes into `nil`, which made `signPlatformAddressOnDemand` surface an empty-derivation-path corruption as the unrelated `.platformAddressNotFound` error — diagnostically misleading. - `signPlatformAddressOnDemand` now switches on the new enum so `.rowMatchedButPathEmpty` surfaces as the existing typed `.derivationPathMissing(addressHashHex:)` error (which had been declared but unreachable). - `canSign` still treats both failure modes uniformly (it doesn't need the diagnostic distinction — the question there is just "can we sign yes/no"). ManagedPlatformWallet.prePersistIdentityKeysForRegistration (CodeRabbit Major): - The build-IdentityPubkey loop no longer silently constructs an empty `Data()` for rows where the FFI returned a null / zero-length `public_key`. This would have created an unusable IdentityPubkey that surfaced much later as a confused registration / signing failure with no breadcrumb to the actually-broken row. Now throws `PlatformWalletError.walletOperation` with the offending row index — symmetric with the existing unknown-enum-byte and count-mismatch guards added in the prior fix pass. Verification: ./build_ios.sh --target sim builds clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../SwiftDashSDK/FFI/KeychainSigner.swift | 61 +++++++++++++------ .../ManagedPlatformWallet.swift | 18 ++++-- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index b68f59aa218..082e5c856e5 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -330,7 +330,14 @@ public final class KeychainSigner: Signer, @unchecked Sendable { if keyType == Self.platformAddressHashKeyType { // Resolve the address row first (synchronous lookup); // mnemonic check is gated on having the wallet id. - guard let resolved = resolvePlatformAddressContext(addressHash: publicKey) else { + // For `canSign` purposes, both "no row" and "corrupt + // row with empty path" mean the same thing: we can't + // sign for this address. The richer signing path + // (`signPlatformAddressOnDemand`) distinguishes them + // for diagnostic reasons. + guard case .found(let resolved) = + resolvePlatformAddressContext(addressHash: publicKey) + else { return false } // `WalletStorage.retrieveMnemonic` throws on miss; @@ -372,21 +379,26 @@ public final class KeychainSigner: Signer, @unchecked Sendable { let derivationPath: String } + /// Result of a `resolvePlatformAddressContext` lookup. + /// Distinguishes "no row matched" (the caller surfaces this as + /// `.platformAddressNotFound` — e.g. address pool not yet + /// synced) from "row exists but its `derivationPath` is empty" + /// (a corrupt persister write — surfaced as + /// `.derivationPathMissing` so the failure is diagnosable). + fileprivate enum PlatformAddressResolution { + case found(PlatformAddressContext) + case noMatch + case rowMatchedButPathEmpty + } + /// SwiftData lookup: 20-byte address hash → /// `(walletId, derivationPath)`. Pinned to the signer's serial /// queue + a per-call private `ModelContext` so the fetch is safe /// off the main actor. - /// - /// Returns `nil` for two distinct reasons (collapsed for the - /// caller's convenience, since both end the request the same - /// way): no row matched the hash, OR the matched row had an - /// empty derivation path (which would indicate a corrupt - /// persister write — `PersistentPlatformAddress.derivationPath` - /// is non-optional and should always carry a DIP-17 path). fileprivate func resolvePlatformAddressContext( addressHash: Data - ) -> PlatformAddressContext? { - var resolved: PlatformAddressContext? = nil + ) -> PlatformAddressResolution { + var resolved: PlatformAddressResolution = .noMatch queue.sync { let context = ModelContext(self.modelContainer) let descriptor = FetchDescriptor( @@ -395,18 +407,23 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } ) guard let row = try? context.fetch(descriptor).first else { + resolved = .noMatch return } - // Empty path = corrupt write; treat as unresolvable so - // the caller surfaces a precise error rather than handing - // an empty path to the FFI. + // Empty path = corrupt persister write + // (`PersistentPlatformAddress.derivationPath` is + // non-optional and should always carry a DIP-17 path). + // Surface as a distinct case so the caller can map to + // `.derivationPathMissing` rather than a misleading + // `.platformAddressNotFound`. guard !row.derivationPath.isEmpty else { + resolved = .rowMatchedButPathEmpty return } - resolved = PlatformAddressContext( + resolved = .found(PlatformAddressContext( walletId: row.walletId, derivationPath: row.derivationPath - ) + )) } return resolved } @@ -435,9 +452,19 @@ public final class KeychainSigner: Signer, @unchecked Sendable { // 1. Look up `(walletId, derivationPath)` on a private // background ModelContext (off the main actor). Mnemonic // fetch is now Rust's job (via the resolver callback), - // not Swift's. - guard let ctx = resolvePlatformAddressContext(addressHash: addressHash) else { + // not Swift's. The two failure modes — "no row matched" + // and "row exists but path is empty" — surface as + // distinct typed errors so a corrupt persister write is + // diagnosable rather than masquerading as an unsynced + // address pool. + let ctx: PlatformAddressContext + switch resolvePlatformAddressContext(addressHash: addressHash) { + case .found(let resolved): + ctx = resolved + case .noMatch: return .failure(.platformAddressNotFound(addressHashHex: hashHex)) + case .rowMatchedButPathEmpty: + return .failure(.derivationPathMissing(addressHashHex: hashHex)) } // Validate the wallet-id length BEFORE shipping the diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index 2fb4a39e65e..c322eedc731 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -772,12 +772,20 @@ extension ManagedPlatformWallet { pubkeys.reserveCapacity(out.count) for i in 0.. 0 { - pubData = Data(bytes: pubPtr, count: row.public_key_len) - } else { - pubData = Data() + // Empty pubkey rows are an FFI contract violation — + // the persist callback already accepted a real pubkey + // for this row, so the parallel out_pubkeys array + // having `nil` / zero-length here means something + // dropped between the persister fire and the row + // serialization. Fail fast with the offending row + // index rather than silently constructing an + // unusable `IdentityPubkey` with empty bytes. + guard let pubPtr = row.public_key, row.public_key_len > 0 else { + throw PlatformWalletError.walletOperation( + "derive_and_persist_identity_keys returned an empty public key for row \(i)" + ) } + let pubData = Data(bytes: pubPtr, count: row.public_key_len) let meta = captured[i] guard let keyType = KeyType(rawValue: meta.keyType), From 69c97f8d2ba1d7ad44aea79fd96e149e7f3c734d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 26 Apr 2026 22:12:58 +0800 Subject: [PATCH 4/5] fix(swift-sdk): keep signer alive across detached FFI work + drop tautological key validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two priority fixes from the third review pass on PR #3542. P1 — non-owning signer ctx could dangle during detached async FFI work (thepastaclaw): The previous fix switched KeychainSigner from `passRetained` to `passUnretained` to remove the leak, but every async wallet API that accepts `signer: KeychainSigner` does its FFI work inside `Task.detached { ... }` and captures only `signer.handle` (an opaque pointer), never the Swift `KeychainSigner` itself. The function parameter holds a strong reference for the duration of the outer `await`, but that's a Swift-implementation detail the optimizer is increasingly free to shorten via "consume" / move-only-types analysis. Under future Swift it could decide `signer` is unused after `let signerHandle = signer.handle` and release it before the detached task fires its first trampoline callback — turning every handle-only call site into a potential use-after-free. Fix: every `Task.detached` closure that passes `signerHandle` to the FFI now begins with `_ = signer` (or `_ = identitySigner; _ = addressSigner` for the dual-signer registration path). That forces Swift to capture the Swift owner strongly into the task closure, keeping the ctx pointer valid for the entire detached work. Documented the lifetime contract on KeychainSigner's type-level "Lifetime contract" note so future call sites follow the same pattern. 11 keepalive lines across 10 functions: - registerIdentityFromAddresses (identitySigner + addressSigner) - registerDpnsName - sendContactRequest - acceptContactRequest - submitDashPayProfileWithSigner - transferCredits - transferCreditsToAddresses - withdrawCredits - updateIdentity - registerIdentityWithFunding P2 — private scalar leaked into non-zeroizable Swift `String` (thepastaclaw): The validation step added in the prior pass copied the 32-byte ECDSA scalar into Swift `Data` (necessary — Keychain takes `Data`) AND additionally hex-encoded it into a Swift `String` just to satisfy the `KeyValidation.validatePrivateKeyForPublicKey(privateKeyHex:...)` API. Swift `String` is heap-allocated and cannot be securely zeroed; the hex copy was a real (small) security regression even though the resulting Keychain row was correct. The validation itself was tautological in this code path: Rust had just derived `publicKey` from `privateKey` via the same `secp256k1` operation a few microseconds earlier, and the `PersistKeyArgs` layout assertion already guards against ABI drift between the two sides. The only realistic failure modes the validation could catch (struct padding drift, byte-order mistake) are already covered by that assertion — so removing the hex String + the validation call has no observable effect except to eliminate the one extra non-zeroizable Swift heap copy of the secret. Done. Verification: ./build_ios.sh --target sim builds clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .agents/skills/pr-description/SKILL.md | 88 +++++++++++++++++++ .../SwiftDashSDK/FFI/KeychainSigner.swift | 27 ++++-- .../FFI/MnemonicResolverAndPersister.swift | 47 ++++------ .../ManagedPlatformWallet.swift | 28 +++++- 4 files changed, 150 insertions(+), 40 deletions(-) create mode 100644 .agents/skills/pr-description/SKILL.md diff --git a/.agents/skills/pr-description/SKILL.md b/.agents/skills/pr-description/SKILL.md new file mode 100644 index 00000000000..ec57684287a --- /dev/null +++ b/.agents/skills/pr-description/SKILL.md @@ -0,0 +1,88 @@ +--- +name: pr-description +description: Generate a pull request description for the current branch. Use when the user asks to create a PR description, write PR body, or prepare a PR for review. +argument-hint: "[base-branch]" +--- + +# Generate PR Description + +Generate a pull request title and description for the current branch using the project's standard template. + +## Steps + +1. Determine the base branch: + - Use the argument if provided + - Otherwise, auto-detect: `git remote set-head origin --auto >/dev/null 2>&1 && git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/||'` + - Fall back to `v3.1-dev` if the command above fails + +2. Gather context by running these git commands: + - `git log --oneline $(git merge-base HEAD )..HEAD` — all commits on this branch + - `git diff ...HEAD --stat` — files changed summary + - `git diff ...HEAD` — full diff (read selectively for large diffs) + +3. Analyze the changes to understand: + - What problem is being solved or feature added + - What specific code changes were made + - Whether there are breaking changes + - What tests were added or modified + +4. Output a suggested PR title using conventional commits format: + - Scopes: `sdk`, `drive`, `dpp`, `dapi`, `dashmate`, `wasm-dpp`, `wasm-sdk`, `platform` + - Types: `feat`, `fix`, `refactor`, `chore`, `docs`, `test`, `build` + - Add `!` after the type for breaking changes (e.g. `feat!:`) + - Format: **Suggested title:** `type(scope): description` + +5. Fill in this PR template (preserve all HTML comments exactly as shown): + +```markdown + + + +## Issue being fixed or feature implemented + + + + + +## What was done? + + + + +## How Has This Been Tested? + + + + + + +## Breaking Changes + + + + + +## Checklist: + +- [ ] I have performed a self-review of my own code +- [ ] I have commented my code, particularly in hard-to-understand areas +- [ ] I have added or updated relevant unit/integration/functional/e2e tests +- [ ] I have added "!" to the title and described breaking changes in the corresponding section if my code contains any +- [ ] I have made corresponding changes to the documentation if needed + +**For repository code-owners and collaborators only** +- [ ] I have assigned this pull request to a milestone +``` + +## Output Format + +Output the entire PR description (title + body) as a single raw Markdown code block so the user can easily copy and paste it. Wrap the output in triple backticks with the `Markdown` language tag. + +## Guidelines + +- Keep the description **concise** — avoid walls of text. Prefer short bullet points over paragraphs +- Be specific — reference file paths, struct/function names, and types +- For "How Has This Been Tested?", check `git diff` for new `*test*`, `*spec*` files. Briefly describe what tests cover (1 line per test file), not every individual test case +- For large diffs, focus on architectural changes rather than listing every file +- Group related changes together in "What was done?" using sub-headers if needed +- Avoid repeating the same information across sections diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index 082e5c856e5..349e87488fa 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -53,25 +53,36 @@ import SwiftData /// SwiftData `ModelContext` is not `Sendable` but we serialise access /// to it through a single internal queue. /// -/// # Lifetime +/// # Lifetime contract /// /// `init` allocates a `*mut SignerHandle` via /// `dash_sdk_signer_create_with_ctx` and registers `self` as the /// opaque ctx via `Unmanaged.passUnretained(self).toOpaque()` — /// a non-owning pointer. ARC alone controls when this object -/// deallocates; the caller must keep the `KeychainSigner` -/// instance alive for the duration of any FFI call that -/// captured `handle`. `deinit` calls -/// `dash_sdk_signer_destroy` to free the Rust-side handle/vtable -/// allocations; the destroy trampoline is a no-op (no extra -/// retain to balance). +/// deallocates; the destroy trampoline is a no-op (no extra +/// retain to balance) and `deinit` calls +/// `dash_sdk_signer_destroy` to free the Rust handle/vtable. +/// +/// **Caller responsibility:** the `KeychainSigner` instance +/// MUST stay alive for the duration of any in-flight FFI call +/// that captured the handle. Async wallet APIs that take +/// `signer: KeychainSigner` perform the FFI work inside +/// `Task.detached`; the function parameter holds a strong +/// reference for the whole `await`, but each `Task.detached` +/// closure must additionally do `_ = signer` so the strong +/// reference is captured into the task and survives every +/// possible Swift compiler optimization of "unused after this +/// point". See the `_ = signer` keepalive lines in +/// `ManagedPlatformWallet.swift` call sites for examples. /// /// Earlier revisions of this file used `passRetained`, which /// created a circular ownership: Rust held a +1 retain on `self`, /// the destroy trampoline only released it when the destroy FFI /// fired, and the destroy FFI was only invoked from `deinit` — /// which ARC could never enter while the +1 retain was alive. -/// Result: every `KeychainSigner` instance leaked forever. +/// Result: every `KeychainSigner` instance leaked forever. The +/// current `passUnretained` shape removes the leak at the cost +/// of the explicit keepalive contract above. public final class KeychainSigner: Signer, @unchecked Sendable { // MARK: Public surface diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift index 8285c3cc2f2..b0fe7f78669 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/MnemonicResolverAndPersister.swift @@ -263,36 +263,27 @@ public final class IdentityKeyPersister: @unchecked Sendable { let publicKeyHashHex = publicKeyHashData .map { String(format: "%02x", $0) } .joined() + // The 32 raw private bytes have to enter Swift `Data` to + // hand to `KeychainManager.storeIdentityPrivateKey`, which + // takes `Data` (Keychain APIs are iOS-only and Rust can't + // call SecItemAdd directly). Swift `Data` cannot be + // securely zeroed, so we keep this allocation as the only + // place the secret bytes touch the Swift heap and let it + // drop with the function frame as soon as Keychain has + // taken its copy. + // + // Earlier revisions also hex-encoded the private bytes + // into a `String` to call + // `KeyValidation.validatePrivateKeyForPublicKey`. That + // validation was tautological in this code path — Rust + // had just derived `publicKey` from `privateKey` via the + // same `secp256k1` operation a few microseconds earlier, + // and the `PersistKeyArgs` layout assertion guards + // against ABI drift between the two sides — so the only + // observable effect was an extra non-zeroizable Swift + // `String` of the private scalar. Removed for that reason. let privateKeyData = Data(bytes: privKeyPtr, count: 32) - // Per swift-sdk/CLAUDE.md "Always validate private keys - // match their public keys using - // KeyValidation.validatePrivateKeyForPublicKey". Catches - // any future ABI / payload corruption (e.g. struct - // padding drift, byte-order mistake, wrong-key-shipped - // bug) at the persist boundary instead of letting a - // mismatched row land in the keychain and surface as an - // opaque "signature invalid" error months later. - guard let resolvedKeyType = KeyType(rawValue: a.key_type) else { - return false - } - let privateKeyHex = privateKeyData - .map { String(format: "%02x", $0) } - .joined() - // `isTestnet` only affects WIF address derivation inside - // the validator, not the underlying ECDSA scalar→pubkey - // correspondence we actually care about — pass `true` as - // a stable default. - let validates = KeyValidation.validatePrivateKeyForPublicKey( - privateKeyHex: privateKeyHex, - publicKeyHex: publicKeyHex, - keyType: resolvedKeyType, - isTestnet: true - ) - guard validates else { - return false - } - // Identity-id is unknown pre-registration — Rust will // recompute it from the input addresses at submit time. // Use the marker `pending` so the keychain explorer makes diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index c322eedc731..2c753c3e011 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -281,6 +281,15 @@ public final class ManagedPlatformWallet: @unchecked Sendable { let (idData, identityHandle): (Data, Handle) = try await Task.detached( priority: .userInitiated ) { () -> (Data, Handle) in + // Keepalive — KeychainSigner now uses + // `passUnretained`, so the Rust ctx pointer dangles + // unless we keep the Swift owners alive across this + // detached work. Implicit-capturing `identitySigner` + // and `addressSigner` here forces strong references + // for the duration of the task; the trampoline can + // safely deref the ctx until this closure returns. + _ = identitySigner + _ = addressSigner let ffiInputs = inputs.map { input -> IdentityFundingInputFFI in var hashTuple = hashTupleInit() withUnsafeMutableBytes(of: &hashTuple) { raw in @@ -997,10 +1006,12 @@ extension ManagedPlatformWallet { signer: KeychainSigner ) async throws -> String { let handle = self.handle - // Take the raw signer handle outside the Task — `KeychainSigner` - // owns its handle for its full lifetime (released in `deinit`), - // so the pointer is safe to capture into the detached Task as - // long as the caller keeps `signer` alive across the await. + // Take the raw signer handle outside the Task. `KeychainSigner` + // uses `passUnretained` for its FFI ctx, so the pointer is + // only safe while the Swift `signer` object is alive. The + // explicit `_ = signer` inside the Task keeps it alive for + // the duration of the detached work — see the "Lifetime + // contract" note on `KeychainSigner`. let signerHandle = signer.handle // Capture the 32-byte payload by value into a Sendable // `[UInt8]` so the detached Task can hand a fresh pointer @@ -1009,6 +1020,7 @@ extension ManagedPlatformWallet { Array(UnsafeBufferPointer(start: ptr, count: 32)) } return try await Task.detached(priority: .userInitiated) { () -> String in + _ = signer var outPtr: UnsafeMutablePointer? = nil var error = PlatformWalletFFIError() let result = idBytes.withUnsafeBufferPointer { idBp in @@ -1422,6 +1434,7 @@ extension ManagedPlatformWallet { let requestHandle: Handle = try await Task.detached(priority: .userInitiated) { () -> Handle in + _ = signer var outHandle: Handle = NULL_HANDLE var error = PlatformWalletFFIError() let result: PlatformWalletFFIResult = senderBytes.withUnsafeBufferPointer { @@ -1488,6 +1501,7 @@ extension ManagedPlatformWallet { let establishedHandle: Handle = try await Task.detached( priority: .userInitiated ) { () -> Handle in + _ = signer var outHandle: Handle = NULL_HANDLE var error = PlatformWalletFFIError() let result = platform_wallet_accept_contact_request_with_signer( @@ -1792,6 +1806,7 @@ extension ManagedPlatformWallet { let avatarBytes = update.avatarBytes return try await Task.detached(priority: .userInitiated) { () -> DashPayProfile in + _ = signer var outProfile = dashPayProfileFFIEmpty() var error = PlatformWalletFFIError() @@ -2161,6 +2176,7 @@ extension ManagedPlatformWallet { Array(UnsafeBufferPointer(start: ptr, count: 32)) } try await Task.detached(priority: .userInitiated) { + _ = signer var error = PlatformWalletFFIError() let result = fromBytes.withUnsafeBufferPointer { fromBp -> PlatformWalletFFIResult in toBytes.withUnsafeBufferPointer { toBp in @@ -2228,6 +2244,7 @@ extension ManagedPlatformWallet { } let rows = ffiRows return try await Task.detached(priority: .userInitiated) { () -> UInt64 in + _ = signer var newBalance: UInt64 = 0 var error = PlatformWalletFFIError() let result = fromBytes.withUnsafeBufferPointer { @@ -2266,6 +2283,7 @@ extension ManagedPlatformWallet { Array(UnsafeBufferPointer(start: ptr, count: 32)) } try await Task.detached(priority: .userInitiated) { + _ = signer var error = PlatformWalletFFIError() let result = idBytes.withUnsafeBufferPointer { idBp -> PlatformWalletFFIResult in @@ -2310,6 +2328,7 @@ extension ManagedPlatformWallet { let addPubkeys = addPublicKeys let disableIds = disablePublicKeyIds try await Task.detached(priority: .userInitiated) { + _ = signer var error = PlatformWalletFFIError() // Mirror the registration FFI's pubkey-pinning pattern via // `withPubkeyFFIArray` so each `pubkey_bytes` pointer the @@ -2379,6 +2398,7 @@ extension ManagedPlatformWallet { let pubkeys = identityPubkeys return try await Task.detached(priority: .userInitiated) { () -> (Identifier, ManagedIdentity) in + _ = signer var idTuple: ( UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, From 659e9b8ac8edf11f12039fbefb6b0398b6ea85bb Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 26 Apr 2026 22:13:43 +0800 Subject: [PATCH 5/5] chore: drop stray .agents/skills/pr-description/SKILL.md Accidentally swept in by `git add -A` in the previous commit (69c97f8d2); the file is unrelated to PR #3542 and shouldn't be tracked. Removing it. Co-Authored-By: Claude Opus 4.7 (1M context) --- .agents/skills/pr-description/SKILL.md | 88 -------------------------- 1 file changed, 88 deletions(-) delete mode 100644 .agents/skills/pr-description/SKILL.md diff --git a/.agents/skills/pr-description/SKILL.md b/.agents/skills/pr-description/SKILL.md deleted file mode 100644 index ec57684287a..00000000000 --- a/.agents/skills/pr-description/SKILL.md +++ /dev/null @@ -1,88 +0,0 @@ ---- -name: pr-description -description: Generate a pull request description for the current branch. Use when the user asks to create a PR description, write PR body, or prepare a PR for review. -argument-hint: "[base-branch]" ---- - -# Generate PR Description - -Generate a pull request title and description for the current branch using the project's standard template. - -## Steps - -1. Determine the base branch: - - Use the argument if provided - - Otherwise, auto-detect: `git remote set-head origin --auto >/dev/null 2>&1 && git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/||'` - - Fall back to `v3.1-dev` if the command above fails - -2. Gather context by running these git commands: - - `git log --oneline $(git merge-base HEAD )..HEAD` — all commits on this branch - - `git diff ...HEAD --stat` — files changed summary - - `git diff ...HEAD` — full diff (read selectively for large diffs) - -3. Analyze the changes to understand: - - What problem is being solved or feature added - - What specific code changes were made - - Whether there are breaking changes - - What tests were added or modified - -4. Output a suggested PR title using conventional commits format: - - Scopes: `sdk`, `drive`, `dpp`, `dapi`, `dashmate`, `wasm-dpp`, `wasm-sdk`, `platform` - - Types: `feat`, `fix`, `refactor`, `chore`, `docs`, `test`, `build` - - Add `!` after the type for breaking changes (e.g. `feat!:`) - - Format: **Suggested title:** `type(scope): description` - -5. Fill in this PR template (preserve all HTML comments exactly as shown): - -```markdown - - - -## Issue being fixed or feature implemented - - - - - -## What was done? - - - - -## How Has This Been Tested? - - - - - - -## Breaking Changes - - - - - -## Checklist: - -- [ ] I have performed a self-review of my own code -- [ ] I have commented my code, particularly in hard-to-understand areas -- [ ] I have added or updated relevant unit/integration/functional/e2e tests -- [ ] I have added "!" to the title and described breaking changes in the corresponding section if my code contains any -- [ ] I have made corresponding changes to the documentation if needed - -**For repository code-owners and collaborators only** -- [ ] I have assigned this pull request to a milestone -``` - -## Output Format - -Output the entire PR description (title + body) as a single raw Markdown code block so the user can easily copy and paste it. Wrap the output in triple backticks with the `Markdown` language tag. - -## Guidelines - -- Keep the description **concise** — avoid walls of text. Prefer short bullet points over paragraphs -- Be specific — reference file paths, struct/function names, and types -- For "How Has This Been Tested?", check `git diff` for new `*test*`, `*spec*` files. Briefly describe what tests cover (1 line per test file), not every individual test case -- For large diffs, focus on architectural changes rather than listing every file -- Group related changes together in "What was done?" using sub-headers if needed -- Avoid repeating the same information across sections