Skip to content
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1e83b53
feat(swift-sdk,platform-wallet): wire shielded transfer/unshield/with…
QuantumExplorer May 5, 2026
2180c68
feat(swift-sdk,platform-wallet): wire shield (Platform → Shielded, Ty…
QuantumExplorer May 5, 2026
c15a175
fix(swift-sdk,platform-wallet): hydrate persisted address balances on…
QuantumExplorer May 5, 2026
a8d9b14
fix(platform-wallet-ffi): run shielded proof on worker thread
QuantumExplorer May 5, 2026
dfbb2f8
fix(platform-wallet): surface Platform's per-input view on shield bro…
QuantumExplorer May 6, 2026
6c72239
fix(platform-wallet): reserve fee headroom on shield input 0
QuantumExplorer May 6, 2026
6e4931c
fix(swift-sdk,platform-wallet): address PR review feedback
QuantumExplorer May 6, 2026
3627d0f
fix(platform-wallet): wire shielded spend Merkle witnesses through Sh…
QuantumExplorer May 6, 2026
9211a0d
fix(swift-example-app): per-wallet shielded commitment-tree DB
QuantumExplorer May 6, 2026
3ffce1a
fix(swift-example-app): repoint ShieldedService when opening a wallet…
QuantumExplorer May 6, 2026
4ba2c42
feat(platform-wallet,swift-sdk): multi-account shielded wallet (Desig…
QuantumExplorer May 6, 2026
771b01c
feat(platform-wallet): shielded changeset + per-subwallet restore-on-…
QuantumExplorer May 6, 2026
784ce02
feat(swift-sdk,platform-wallet-ffi): SwiftData persistence for shield…
QuantumExplorer May 6, 2026
589ee68
feat(swift-example-app): storage explorer rows for shielded notes + s…
QuantumExplorer May 6, 2026
9ab48e3
feat(swift-example-app): multi-account shielded UI in WalletDetailView
QuantumExplorer May 6, 2026
2daf333
fix(platform-wallet): derive shielded spend anchor from witness paths
QuantumExplorer May 6, 2026
44f9f57
fix(platform-wallet): use monotonic checkpoint id in shielded sync
QuantumExplorer May 6, 2026
515a694
fix(swift-example-app): route orphan recovery to the wallet's intende…
QuantumExplorer May 6, 2026
a3f4edd
fix(swift-example-app): aggregate orphan-recovery failures with actio…
QuantumExplorer May 6, 2026
00624ad
fix(swift-example-app): log orphan-recovery errors via SDKLogger
QuantumExplorer May 6, 2026
ed30077
fix(swift-sdk,swift-example-app): always log orphan-recovery failures
QuantumExplorer May 6, 2026
83054cb
fix(platform-wallet): validate spend anchor against Platform's record…
QuantumExplorer May 6, 2026
e532eef
fix(platform-wallet,sdk): fall back to most-recent shielded anchor wh…
QuantumExplorer May 6, 2026
9609aee
Merge remote-tracking branch 'origin/v3.1-dev' into platform-wallet/s…
QuantumExplorer May 7, 2026
497b74f
docs(claude): fix iOS framework build path in project CLAUDE.md
QuantumExplorer May 7, 2026
d6a890a
fix(swift-example-app): only overlay shielded-notes empty state when …
QuantumExplorer May 7, 2026
6cc4d9a
fix(swift-example-app): add storage-explorer detail views for shielde…
QuantumExplorer May 7, 2026
1911d73
revert(platform-wallet): drop spend-side anchor pre-flight, trust dep…
QuantumExplorer May 7, 2026
9e4ecdf
feat(swift-example-app): show per-account synced index in shielded sy…
QuantumExplorer May 7, 2026
532b886
fix(swift-example-app): use renamed boundWalletId in shielded sync ev…
QuantumExplorer May 7, 2026
33267c5
fix(swift-example-app): show persisted balance per shielded account, …
QuantumExplorer May 7, 2026
c1bb53a
Merge remote-tracking branch 'origin/v3.1-dev' into platform-wallet/s…
QuantumExplorer May 7, 2026
c1b0eaf
Merge remote-tracking branch 'origin/v3.1-dev' into platform-wallet/s…
QuantumExplorer May 7, 2026
aea6dd1
fix(platform-wallet,swift-sdk): pending-then-confirm shielded spends …
QuantumExplorer May 8, 2026
1e22b90
Merge remote-tracking branch 'origin/v3.1-dev' into platform-wallet/s…
QuantumExplorer May 8, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/rs-platform-wallet-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ pub mod platform_addresses;
pub mod platform_wallet_info;
mod runtime;
#[cfg(feature = "shielded")]
pub mod shielded_send;
#[cfg(feature = "shielded")]
pub mod shielded_sync;
pub mod shielded_types;
pub mod sign_with_mnemonic_resolver;
Expand Down Expand Up @@ -107,6 +109,8 @@ pub use platform_address_types::*;
pub use platform_addresses::*;
pub use platform_wallet_info::*;
#[cfg(feature = "shielded")]
pub use shielded_send::*;
#[cfg(feature = "shielded")]
pub use shielded_sync::*;
pub use shielded_types::*;
pub use sign_with_mnemonic_resolver::*;
Expand Down
338 changes: 338 additions & 0 deletions packages/rs-platform-wallet-ffi/src/shielded_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,338 @@
//! FFI bindings for the shielded spend pipeline (transitions
//! 15/16/17/19 — shield, transfer, unshield, withdraw).
//!
//! Transitions 16/17/19 sign with the bound shielded wallet's
//! Orchard `SpendAuthorizingKey`, which lives on the
//! `OrchardKeySet` cached after [`platform_wallet_manager_bind_shielded`].
//! No host-side `Signer<PlatformAddress>` is required — the host
//! only supplies the recipient + amount (+ core fee rate for
//! withdrawal) and the resulting Halo 2 proof + state transition
//! is built and broadcast on the Rust side.
//!
//! Transition 15 (`shield` — Platform→Shielded) additionally
//! takes a host-supplied `Signer<PlatformAddress>` because the
//! input addresses' ECDSA signatures live in the host keychain.
//! Per-input nonces are fetched from Platform inside
//! [`ShieldedWallet::shield`] before building.
//!
//! Type 18 (`shield_from_asset_lock` — Core L1→Shielded) lives on
//! [`ShieldedWallet`] but isn't wired here yet — it needs the
//! asset-lock proof + private key threaded through.
//!
//! Feature-gated behind `shielded`. The accompanying
//! [`platform_wallet_shielded_warm_up_prover`] entry-point is
//! also defined here so hosts can pre-build the Halo 2 proving
//! key on a background thread at app startup.
//!
//! [`ShieldedWallet`]: platform_wallet::wallet::shielded::ShieldedWallet
//! [`ShieldedWallet::shield`]: platform_wallet::wallet::shielded::ShieldedWallet::shield

use std::ffi::CStr;
use std::os::raw::c_char;

use platform_wallet::wallet::shielded::CachedOrchardProver;
use rs_sdk_ffi::{SignerHandle, VTableSigner};

use crate::check_ptr;
use crate::error::*;
use crate::handle::*;
use crate::runtime::{block_on_worker, runtime};

/// Kick off the Halo 2 proving-key build on a background tokio
/// worker if it hasn't been built yet. Returns immediately —
/// hosts can call this at app startup without blocking the UI
/// thread. Subsequent calls are cheap no-ops once the key is
/// cached. The first shielded send still pays the ~30 s build
/// cost only if it fires before the warm-up worker finishes;
/// `platform_wallet_shielded_prover_is_ready` reports whether
/// that's the case.
///
/// Independent of any manager — the cache is a process-global
/// `OnceLock`.
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_shielded_warm_up_prover() {
runtime().spawn_blocking(|| CachedOrchardProver::new().warm_up());
}

/// Whether the Halo 2 proving key has already been built.
///
/// Useful as a UI indicator ("preparing prover…") before the
/// first shielded send. `false` doesn't mean shielded sends will
/// fail — it just means the next one will pay the ~30s build
/// cost up front.
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_shielded_prover_is_ready() -> bool {
CachedOrchardProver::new().is_ready()
}

/// Send a shielded → shielded transfer.
///
/// Spends notes from `wallet_id`'s shielded balance and creates a
/// new note for `recipient_raw_43`. `amount` is in credits
/// (1 DASH = 1e11 credits). Errors if the wallet has no bound
/// shielded sub-wallet, no spendable notes, or insufficient
/// shielded balance to cover `amount + estimated_fee`.
///
/// # Safety
/// - `wallet_id_bytes` must point to 32 readable bytes.
/// - `recipient_raw_43` must point to 43 readable bytes (the
/// recipient's raw Orchard payment address — same shape
/// `platform_wallet_manager_shielded_default_address` returns).
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_manager_shielded_transfer(
handle: Handle,
wallet_id_bytes: *const u8,
account: u32,
recipient_raw_43: *const u8,
amount: u64,
) -> PlatformWalletFFIResult {
check_ptr!(wallet_id_bytes);
check_ptr!(recipient_raw_43);

let mut wallet_id = [0u8; 32];
std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32);
let mut recipient = [0u8; 43];
std::ptr::copy_nonoverlapping(recipient_raw_43, recipient.as_mut_ptr(), 43);

let wallet = match resolve_wallet(handle, &wallet_id) {
Ok(w) => w,
Err(result) => return result,
};

// Run the proof on a worker thread (8 MB stack). Halo 2 circuit
// synthesis recurses past the ~512 KB iOS dispatch-thread stack
// and crashes with EXC_BAD_ACCESS at the first
// `synthesize(... measure(pass))` call when polled on the
// calling thread.
let result = block_on_worker(async move {
let prover = CachedOrchardProver::new();
wallet
.shielded_transfer_to(account, &recipient, amount, &prover)
.await
});
if let Err(e) = result {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
format!("shielded transfer failed: {e}"),
);
}
PlatformWalletFFIResult::ok()
}

/// Unshield: spend shielded notes and send `amount` credits to a
/// platform address.
///
/// `to_platform_addr_cstr` is the recipient as a NUL-terminated
/// UTF-8 bech32m string (e.g. `"dash1..."` on mainnet,
/// `"tdash1..."` on testnet). The Rust side parses it via
/// `PlatformAddress::from_bech32m_string` so hosts don't have to
/// hand-roll the bincode storage variant tag (`0x00`/`0x01`),
/// which differs from the bech32m payload's type byte
/// (`0xb0`/`0x80`).
///
/// # Safety
/// - `wallet_id_bytes` must point to 32 readable bytes.
/// - `to_platform_addr_cstr` must be a valid NUL-terminated UTF-8
/// C string for the duration of the call.
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_manager_shielded_unshield(
handle: Handle,
wallet_id_bytes: *const u8,
account: u32,
to_platform_addr_cstr: *const c_char,
amount: u64,
) -> PlatformWalletFFIResult {
check_ptr!(wallet_id_bytes);
check_ptr!(to_platform_addr_cstr);

let mut wallet_id = [0u8; 32];
std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32);
let to_addr_str = match CStr::from_ptr(to_platform_addr_cstr).to_str() {
Ok(s) => s.to_string(),
Err(e) => {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorUtf8Conversion,
format!("to_platform_addr is not valid UTF-8: {e}"),
);
}
};

let wallet = match resolve_wallet(handle, &wallet_id) {
Ok(w) => w,
Err(result) => return result,
};

let result = block_on_worker(async move {
let prover = CachedOrchardProver::new();
wallet
.shielded_unshield_to(account, &to_addr_str, amount, &prover)
.await
});
if let Err(e) = result {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
format!("shielded unshield failed: {e}"),
);
}
PlatformWalletFFIResult::ok()
}

/// Withdraw: spend shielded notes and send `amount` credits to a
/// Core L1 address. `to_core_address_cstr` is the address as a
/// Base58Check NUL-terminated UTF-8 string (e.g.
/// `"yL...."` on testnet); the Rust side parses it and verifies
/// the network matches the wallet's. `core_fee_per_byte` is the
/// L1 fee rate in duffs/byte (`1` is the dashmate default).
///
/// # Safety
/// - `wallet_id_bytes` must point to 32 readable bytes.
/// - `to_core_address_cstr` must be a valid NUL-terminated UTF-8
/// C string for the duration of the call.
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_manager_shielded_withdraw(
handle: Handle,
wallet_id_bytes: *const u8,
account: u32,
to_core_address_cstr: *const c_char,
amount: u64,
core_fee_per_byte: u32,
) -> PlatformWalletFFIResult {
check_ptr!(wallet_id_bytes);
check_ptr!(to_core_address_cstr);

let mut wallet_id = [0u8; 32];
std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32);
let to_core = match CStr::from_ptr(to_core_address_cstr).to_str() {
Ok(s) => s.to_string(),
Err(e) => {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorUtf8Conversion,
format!("to_core_address is not valid UTF-8: {e}"),
);
}
};

let wallet = match resolve_wallet(handle, &wallet_id) {
Ok(w) => w,
Err(result) => return result,
};

let result = block_on_worker(async move {
let prover = CachedOrchardProver::new();
wallet
.shielded_withdraw_to(account, &to_core, amount, core_fee_per_byte, &prover)
.await
});
if let Err(e) = result {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
format!("shielded withdraw failed: {e}"),
);
}
PlatformWalletFFIResult::ok()
}

/// Shield: spend credits from a Platform Payment account into
/// the bound shielded sub-wallet's pool.
///
/// `shielded_account` selects which ZIP-32 Orchard account on
/// the bound shielded sub-wallet receives the new note.
/// `payment_account` selects which Platform Payment account on
/// the transparent side funds the shield (auto-selects input
/// addresses in ascending derivation order until the cumulative
/// balance covers `amount + fee buffer`).
///
/// `signer_address_handle` is a `*mut SignerHandle` produced by
/// `dash_sdk_signer_create_with_ctx` (typically Swift's
/// `KeychainSigner.handle`). The caller retains ownership; this
/// function does not destroy the handle.
///
/// # Safety
/// - `wallet_id_bytes` must point to 32 readable bytes.
/// - `signer_address_handle` must be a valid, non-destroyed
/// `*mut SignerHandle` that outlives this call and points at a
/// `VTableSigner` with the callback variant (the native variant
/// doesn't satisfy `Signer<PlatformAddress>`).
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_manager_shielded_shield(
handle: Handle,
wallet_id_bytes: *const u8,
shielded_account: u32,
payment_account: u32,
amount: u64,
signer_address_handle: *mut SignerHandle,
) -> PlatformWalletFFIResult {
Comment on lines +256 to +264
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: signer_address_handle is read-only — prefer *const SignerHandle

The doc comment at lines 245-248 explicitly states the caller retains ownership and the function does not destroy the handle, and the body only reads through it (the cast on line 285 is as *const VTableSigner). Taking *mut SignerHandle invites callers to read this signature as 'may mutate or take ownership', the opposite of the documented contract. *const SignerHandle matches the actual semantics and is consistent with how the rest of the workspace consumes a non-destroying signer reference (e.g. rs-sdk-ffi/src/token/*.rs). Pure FFI ergonomics — no ABI impact.

source: ['claude']

check_ptr!(wallet_id_bytes);
check_ptr!(signer_address_handle);
Comment on lines +256 to +266
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*💬 Nitpick: signer_address_handle is read-only — prefer const SignerHandle

The header explicitly states the caller retains ownership and the function does not destroy the handle, and the body only reads through it. Taking *mut SignerHandle invites callers to think the function may mutate or take ownership; *const SignerHandle matches the actual contract. Pure FFI ergonomics — no behavioural change.

source: ['claude']


let mut wallet_id = [0u8; 32];
std::ptr::copy_nonoverlapping(wallet_id_bytes, wallet_id.as_mut_ptr(), 32);

let wallet = match resolve_wallet(handle, &wallet_id) {
Ok(w) => w,
Err(result) => return result,
};

// SAFETY: the caller retains ownership of the signer handle
// and guarantees it outlives this call. We block until the
// worker future completes, so the `'static` lifetime we paint
// on the borrow does not actually outlive the host's handle.
// `VTableSigner` is `Send + Sync` per its `unsafe impl` in
// rs-sdk-ffi, so `&'static VTableSigner` is automatically
// `Send + 'static` — exactly what `block_on_worker` needs.
let address_signer: &'static VTableSigner =
std::mem::transmute::<&VTableSigner, &'static VTableSigner>(
&*(signer_address_handle as *const VTableSigner),
);
Comment on lines +283 to +286
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Use the established usize round-trip pattern instead of transmuting the signer borrow to 'static

block_on_worker requires F: 'static, and the new shield path satisfies this with mem::transmute::<&VTableSigner, &'static VTableSigner>(...). It is sound today only because block_on_worker (runtime.rs) parks on the spawned future to completion — any future change that lets it return early (timeout, cancellation, shutdown select!) silently turns this into a use-after-free. Other call sites in this crate (e.g. identity_top_up.rs:117-122) solve the same Send + 'static constraint by round-tripping the signer pointer through usize and re-materializing the &VTableSigner inside the future, which captures only Send + 'static data and avoids the lifetime fiction entirely. Aligning the shield path to that pattern would remove a sharp edge from the FFI surface at zero behavioural cost.

source: ['claude']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- [SUGGESTION] lines 268-271: Use the established usize round-trip pattern instead of transmuting the signer borrow to 'static
  `block_on_worker` requires `F: 'static`, and the new shield path satisfies this with `mem::transmute::<&VTableSigner, &'static VTableSigner>(...)`. It is sound today only because `block_on_worker` (`runtime.rs`) parks on the spawned future to completion — any future change that lets it return early (timeout, cancellation, shutdown select!) silently turns this into a use-after-free. Other call sites in this crate (e.g. `identity_top_up.rs:117-122`) solve the same `Send + 'static` constraint by round-tripping the signer pointer through `usize` and re-materializing the `&VTableSigner` *inside* the future, which captures only `Send + 'static` data and avoids the lifetime fiction entirely. Aligning the shield path to that pattern would remove a sharp edge from the FFI surface at zero behavioural cost.

Comment on lines +276 to +286
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Use the existing usize round-trip instead of forging a &'static borrow of the host signer

platform_wallet_manager_shielded_shield does mem::transmute::<&VTableSigner, &'static VTableSigner>(&*(signer_address_handle as *const VTableSigner)) to satisfy block_on_worker's Send + 'static bound. It is sound today only because block_on_worker (runtime.rs:49-56) is rt.block_on(async move { rt.spawn(future).await.expect(...) }) — the calling thread parks until the spawned task completes, so the host-owned SignerHandle outlives the borrow. Any future change that lets block_on_worker return early (a shutdown select!, a timeout, a cancellation token, replacing .expect with a ? early-return) silently turns this into a use-after-free across the FFI boundary, since Swift's KeychainSigner.deinit is free to destroy the handle once the FFI call returns. The Swift wrapper's _ = addressSigner keep-alive in the detached task only covers the duration of the FFI call. The same crate already solves this exact constraint without lifetime fiction at identity_top_up.rs:113-126: round-trip the pointer through usize and re-materialize &VTableSigner inside the spawned future, so the future captures only Send + 'static data (usize) and the borrow is created freshly each poll. Aligning the shield path with that precedent removes the unsafe { transmute } from the FFI surface at zero behavioural cost and decouples it from block_on_worker's implementation details.

source: ['claude']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- [SUGGESTION] lines 276-286: Use the existing `usize` round-trip instead of forging a `&'static` borrow of the host signer
  `platform_wallet_manager_shielded_shield` does `mem::transmute::<&VTableSigner, &'static VTableSigner>(&*(signer_address_handle as *const VTableSigner))` to satisfy `block_on_worker`'s `Send + 'static` bound. It is sound today only because `block_on_worker` (`runtime.rs:49-56`) is `rt.block_on(async move { rt.spawn(future).await.expect(...) })` — the calling thread parks until the spawned task completes, so the host-owned `SignerHandle` outlives the borrow. Any future change that lets `block_on_worker` return early (a shutdown `select!`, a timeout, a cancellation token, replacing `.expect` with a `?` early-return) silently turns this into a use-after-free across the FFI boundary, since Swift's `KeychainSigner.deinit` is free to destroy the handle once the FFI call returns. The Swift wrapper's `_ = addressSigner` keep-alive in the detached task only covers the duration of the FFI call. The same crate already solves this exact constraint without lifetime fiction at `identity_top_up.rs:113-126`: round-trip the pointer through `usize` and re-materialize `&VTableSigner` *inside* the spawned future, so the future captures only `Send + 'static` data (`usize`) and the borrow is created freshly each poll. Aligning the shield path with that precedent removes the `unsafe { transmute }` from the FFI surface at zero behavioural cost and decouples it from `block_on_worker`'s implementation details.


// Run the proof on a worker thread (8 MB stack). Halo 2 circuit
// synthesis recurses past the ~512 KB iOS dispatch-thread stack
// and crashes with EXC_BAD_ACCESS at the first
// `synthesize(... measure(pass))` call when polled on the
// calling thread.
let result = block_on_worker(async move {
let prover = CachedOrchardProver::new();
wallet
.shielded_shield_from_account(
shielded_account,
payment_account,
amount,
address_signer,
Comment on lines +276 to +300
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: The shield FFI path fabricates a 'static signer borrow to satisfy the spawned future

platform_wallet_manager_shielded_shield converts the host-owned signer handle into &VTableSigner and then uses transmute to treat that borrow as &'static VTableSigner before passing it into block_on_worker. This is only sound under the current exact implementation of block_on_worker, which blocks until rt.spawn(future) completes. If that helper ever changes to allow early return on timeout, cancellation, shutdown, or different error propagation, Rust will still hold a forged 'static reference after the FFI call returns, while Swift is free to destroy the KeychainSigner handle. The same crate already has the safer pattern in identity_top_up.rs: capture the pointer as a usize, then reconstruct the borrow inside the spawned future so only genuinely Send + 'static data crosses the boundary.

💡 Suggested change
Suggested change
// SAFETY: the caller retains ownership of the signer handle
// and guarantees it outlives this call. We block until the
// worker future completes, so the `'static` lifetime we paint
// on the borrow does not actually outlive the host's handle.
// `VTableSigner` is `Send + Sync` per its `unsafe impl` in
// rs-sdk-ffi, so `&'static VTableSigner` is automatically
// `Send + 'static` — exactly what `block_on_worker` needs.
let address_signer: &'static VTableSigner =
std::mem::transmute::<&VTableSigner, &'static VTableSigner>(
&*(signer_address_handle as *const VTableSigner),
);
// Run the proof on a worker thread (8 MB stack). Halo 2 circuit
// synthesis recurses past the ~512 KB iOS dispatch-thread stack
// and crashes with EXC_BAD_ACCESS at the first
// `synthesize(... measure(pass))` call when polled on the
// calling thread.
let result = block_on_worker(async move {
let prover = CachedOrchardProver::new();
wallet
.shielded_shield_from_account(
shielded_account,
payment_account,
amount,
address_signer,
let signer_addr = signer_address_handle as usize;
let result = block_on_worker(async move {
let address_signer: &VTableSigner = unsafe { &*(signer_addr as *const VTableSigner) };
let prover = CachedOrchardProver::new();
wallet
.shielded_shield_from_account(
shielded_account,
payment_account,
amount,
address_signer,
&prover,
)
.await
});

source: ['claude', 'codex']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- [SUGGESTION] lines 276-300: The shield FFI path fabricates a `'static` signer borrow to satisfy the spawned future
  `platform_wallet_manager_shielded_shield` converts the host-owned signer handle into `&VTableSigner` and then uses `transmute` to treat that borrow as `&'static VTableSigner` before passing it into `block_on_worker`. This is only sound under the current exact implementation of `block_on_worker`, which blocks until `rt.spawn(future)` completes. If that helper ever changes to allow early return on timeout, cancellation, shutdown, or different error propagation, Rust will still hold a forged `'static` reference after the FFI call returns, while Swift is free to destroy the `KeychainSigner` handle. The same crate already has the safer pattern in `identity_top_up.rs`: capture the pointer as a `usize`, then reconstruct the borrow inside the spawned future so only genuinely `Send + 'static` data crosses the boundary.

&prover,
)
.await
});
Comment on lines +276 to +304
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Use the established usize round-trip pattern instead of transmuting the signer borrow to &'static

block_on_worker requires F: Send + 'static (runtime.rs:49-56), and the new shield path satisfies that with mem::transmute::<&VTableSigner, &'static VTableSigner>(...) at lines 276-279. It is sound today only because block_on_worker is rt.block_on(async move { rt.spawn(future).await.expect(...) }) — the calling thread parks until the spawned task completes, so the host-owned SignerHandle outlives the borrow. Any future change that lets block_on_worker return early — a shutdown select!, a timeout, a cancellation token, or replacing .expect with a ?-style return — silently turns this into a use-after-free across the FFI boundary, since Swift's KeychainSigner.deinit is free to destroy the handle as soon as the FFI call returns. The same crate already solves the identical constraint without the lifetime fiction: identity_top_up.rs:113-126 round-trips the pointer through usize and re-materializes &VTableSigner inside the spawned future (capturing only Send + 'static data). Aligning the shield path with that precedent removes the unsafe { transmute } from the FFI surface at zero behavioural cost.

💡 Suggested change
Suggested change
// SAFETY: the caller retains ownership of the signer handle
// and guarantees it outlives this call. We block until the
// worker future completes, so the `'static` lifetime we paint
// on the borrow does not actually outlive the host's handle.
// `VTableSigner` is `Send + Sync` per its `unsafe impl` in
// rs-sdk-ffi, so `&'static VTableSigner` is automatically
// `Send + 'static` — exactly what `block_on_worker` needs.
let address_signer: &'static VTableSigner =
std::mem::transmute::<&VTableSigner, &'static VTableSigner>(
&*(signer_address_handle as *const VTableSigner),
);
// Run the proof on a worker thread (8 MB stack). Halo 2 circuit
// synthesis recurses past the ~512 KB iOS dispatch-thread stack
// and crashes with EXC_BAD_ACCESS at the first
// `synthesize(... measure(pass))` call when polled on the
// calling thread.
let result = block_on_worker(async move {
let prover = CachedOrchardProver::new();
wallet
.shielded_shield_from_account(account_index, amount, address_signer, &prover)
.await
});
// Round-trip the signer pointer through `usize` so the spawned
// future's capture is `Send + 'static` (the raw pointer is `!Send`,
// but `usize` is). The underlying `Inner::Callback { ctx, vtable }`
// is `Send + Sync` — see the unsafe impls in `rs-sdk-ffi/src/signer.rs`.
let signer_addr = signer_address_handle as usize;
// Run the proof on a worker thread (8 MB stack). Halo 2 circuit
// synthesis recurses past the ~512 KB iOS dispatch-thread stack
// and crashes with EXC_BAD_ACCESS at the first
// `synthesize(... measure(pass))` call when polled on the
// calling thread.
let result = block_on_worker(async move {
let address_signer: &VTableSigner = unsafe { &*(signer_addr as *const VTableSigner) };
let prover = CachedOrchardProver::new();
wallet
.shielded_shield_from_account(account_index, amount, address_signer, &prover)
.await
});

source: ['claude', 'codex']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-ffi/src/shielded_send.rs`:
- [SUGGESTION] lines 269-291: Use the established usize round-trip pattern instead of transmuting the signer borrow to &'static
  `block_on_worker` requires `F: Send + 'static` (runtime.rs:49-56), and the new shield path satisfies that with `mem::transmute::<&VTableSigner, &'static VTableSigner>(...)` at lines 276-279. It is sound today only because `block_on_worker` is `rt.block_on(async move { rt.spawn(future).await.expect(...) })` — the calling thread parks until the spawned task completes, so the host-owned `SignerHandle` outlives the borrow. Any future change that lets `block_on_worker` return early — a shutdown `select!`, a timeout, a cancellation token, or replacing `.expect` with a `?`-style return — silently turns this into a use-after-free across the FFI boundary, since Swift's `KeychainSigner.deinit` is free to destroy the handle as soon as the FFI call returns. The same crate already solves the identical constraint without the lifetime fiction: `identity_top_up.rs:113-126` round-trips the pointer through `usize` and re-materializes `&VTableSigner` *inside* the spawned future (capturing only `Send + 'static` data). Aligning the shield path with that precedent removes the `unsafe { transmute }` from the FFI surface at zero behavioural cost.

if let Err(e) = result {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
format!("shielded shield failed: {e}"),
);
}
PlatformWalletFFIResult::ok()
}

/// Resolve the wallet `Arc` for the given manager handle, or
/// produce a `PlatformWalletFFIResult` describing why we couldn't.
fn resolve_wallet(
handle: Handle,
wallet_id: &[u8; 32],
) -> Result<std::sync::Arc<platform_wallet::PlatformWallet>, PlatformWalletFFIResult> {
let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
runtime().block_on(manager.get_wallet(wallet_id))
});
let inner_option = match option {
Some(v) => v,
None => {
return Err(PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorInvalidHandle,
format!("invalid manager handle: {handle}"),
));
}
};
inner_option.ok_or_else(|| {
PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
format!("wallet not found: {}", hex::encode(wallet_id)),
)
})
}
Loading
Loading