Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions packages/rs-platform-wallet-ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,18 @@ impl From<PlatformWalletError> for PlatformWalletFFIResult {
PlatformWalletError::PlatformShieldCapacityExceeded { .. } => {
PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance
}
// The per-input sibling of the account-capacity variant above: a
// live pre-broadcast per-input shortfall (a stale-snapshot race).
// It rides the SAME capacity code — the host's corrective action is
// identical (refresh preflight, retry) — but as its OWN wallet
// variant so the message names the offending address and the typed
// `available`/`required` are understood as that single input's live
// figures, not an account maximum. Minting a distinct FFI code was
// deliberately avoided to not collide with the in-flight code-space
// frontier; the disambiguation lives in the message.
PlatformWalletError::PlatformShieldInputShortfall { .. } => {
PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance
}
// The core-transaction sibling of the shielded pair above: the
// do-not-retry signal must survive the boundary as a typed code
// so hosts can distinguish it from a definitive rejection.
Expand Down Expand Up @@ -708,6 +720,14 @@ impl From<PlatformWalletError> for PlatformWalletFFIResult {
// `MessageSigningKeyUnavailable` mapped above. By the time a
// `MessageSigningFailed` reason exists, any marker in it sits
// mid-string and is deliberately not matched.
// A panic recovered at the runtime-helper boundary
// (`runtime::block_on_worker` / `run_on_big_stack_thread`) instead
// of aborting the host. It is by definition an unexpected internal
// failure, so it reuses the generic `ErrorUnknown` code (per the
// "add or reuse an internal-panic code" contract); the panic text
// rides the message. An explicit arm — rather than the catch-all
// below — so the deliberate reuse is greppable.
PlatformWalletError::InternalPanic(..) => PlatformWalletFFIResultCode::ErrorUnknown,
_ => PlatformWalletFFIResultCode::ErrorUnknown,
};
PlatformWalletFFIResult::err(code, error.to_string())
Expand Down
197 changes: 191 additions & 6 deletions packages/rs-platform-wallet-ffi/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,144 @@ pub(crate) fn runtime() -> &'static tokio::runtime::Runtime {
&RT
}

/// Convert a caught worker/thread panic into a value of the driven future's
/// own output type, so a panic surfaces as a typed error at the FFI boundary
/// instead of unwinding across `extern "C"` and aborting the host (workspace
/// policy — `Cargo.toml`: "a JNI library must never abort the app process").
///
/// Implemented for every output type actually driven through
/// [`block_on_worker`]: any `Result<T, E>` whose error type recovers (the
/// overwhelmingly common shape — the panic becomes a typed `Err`), plus the
/// handful of non-`Result`, best-effort outputs whose panic degrades to a
/// logged, empty value. The [`block_on_worker`] bound is fail-closed: a new
/// call site whose output does not implement this will not compile until it
/// opts into an explicit recovery here.
pub(crate) trait RecoverWorkerPanic {
fn recover_from_worker_panic(reason: String) -> Self;
}

impl<T, E: RecoverWorkerPanic> RecoverWorkerPanic for Result<T, E> {
fn recover_from_worker_panic(reason: String) -> Self {
Err(E::recover_from_worker_panic(reason))
}
}

impl RecoverWorkerPanic for platform_wallet::PlatformWalletError {
fn recover_from_worker_panic(reason: String) -> Self {
platform_wallet::PlatformWalletError::InternalPanic(reason)
}
}

/// Fire-and-forget `..._sync_now` entry points returning `()`: the panic is
/// already logged by the runtime helper; the sync is a no-op for this pass and
/// the host's next periodic sync retries.
impl RecoverWorkerPanic for () {
fn recover_from_worker_panic(_reason: String) -> Self {}
}

/// Contact-crypto counters: a recovered panic reports zero (logged), which the
/// host reconciles on its next sync — never an abort.
impl RecoverWorkerPanic for usize {
fn recover_from_worker_panic(_reason: String) -> Self {
0
}
}

/// Best-effort sync summaries: a recovered panic yields an empty summary
/// (0 processed / 0 errors), logged; the host's next sync retries. Never an
/// abort.
impl RecoverWorkerPanic for platform_wallet::DashPaySyncSummary {
fn recover_from_worker_panic(_reason: String) -> Self {
Self::default()
}
}

impl RecoverWorkerPanic for platform_wallet::manager::dpns_sync::DpnsSyncPassSummary {
fn recover_from_worker_panic(_reason: String) -> Self {
Self::default()
}
}

impl RecoverWorkerPanic for platform_wallet::manager::shielded_sync::ShieldedSyncPassSummary {
fn recover_from_worker_panic(_reason: String) -> Self {
Self::default()
}
}

/// Best-effort message from a caught panic payload (`Box<dyn Any>`), which is a
/// `&'static str` or `String` for essentially every panic (`panic!`, `unwrap`,
/// `expect`, assertions).
fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String {
if let Some(s) = payload.downcast_ref::<&'static str>() {
(*s).to_string()
} else if let Some(s) = payload.downcast_ref::<String>() {
s.clone()
} else {
"non-string panic payload".to_string()
}
}

/// Drive `future` to completion, moving the actual polling onto a
/// worker thread so the caller's stack size doesn't bound the
/// computation.
///
/// The calling thread still blocks (that's what FFI wants); it just
/// parks on a oneshot instead of driving the future itself.
///
/// ## Panic safety
///
/// A panic inside `future` must NOT cross the `extern "C"` FFI boundary: on the
/// `unwind` (Android/host) build that unwind aborts the process with `SIGABRT`
/// (Rust aborts when a panic escapes an `extern "C"` fn), and the JNI shim's
/// own `catch_unwind` (`rs-unified-sdk-jni`) sits ABOVE this callee, so it can
/// never intercept the abort — exactly what the workspace policy in `Cargo.toml`
/// forbids. Two panic surfaces are closed here:
///
/// 1. `future` panics — tokio unwind-catches the spawned task into a
/// `JoinError`. The pre-fix `.expect("tokio worker panicked")` re-raised it
/// (that re-panic was the abort). We instead convert it into the output
/// type's typed error via [`RecoverWorkerPanic`].
/// 2. Any stray panic while `block_on` drives the `JoinHandle` — caught by the
/// surrounding `catch_unwind` and recovered the same way.
///
/// On the iOS `panic = "abort"` profiles (`dev-ios` / `release-ios`) this is
/// INERT by design: the process aborts at the panic site before any
/// `catch_unwind`/`JoinError` is observed. That matches the in-tree note that
/// `catch_unwind` cannot protect an abort-configured build; this hardens the
/// `unwind` builds without pretending to protect iOS.
///
/// The success path is unchanged: a future that completes normally returns its
/// value directly.
pub(crate) fn block_on_worker<F>(future: F) -> F::Output
where
F: std::future::Future + Send + 'static,
F::Output: Send + 'static,
F::Output: Send + 'static + RecoverWorkerPanic,
{
let rt = runtime();
rt.block_on(async move { rt.spawn(future).await.expect("tokio worker panicked") })
let joined = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
rt.block_on(async move { rt.spawn(future).await })
}));
match joined {
// Success path (unchanged): the task completed and returned its value.
Ok(Ok(output)) => output,
// The spawned task panicked (or was cancelled): tokio captured it as a
// `JoinError`. Recover into the output's typed error instead of
// re-raising it across the `extern "C"` caller.
Ok(Err(join_err)) => {
let reason = format!("tokio worker task did not complete: {join_err}");
tracing::error!(target: "platform_wallet_ffi", "{reason}");
F::Output::recover_from_worker_panic(reason)
}
// Belt and suspenders: a panic in the `block_on` driver itself.
Err(panic_payload) => {
let reason = format!(
"panic while driving tokio worker: {}",
panic_payload_message(panic_payload.as_ref())
);
tracing::error!(target: "platform_wallet_ffi", "{reason}");
F::Output::recover_from_worker_panic(reason)
}
}
}

/// Run `f` to completion on a freshly spawned scoped OS thread with the
Expand All @@ -76,16 +201,34 @@ where
/// compiles: it reuses pooled runtime workers instead of paying a
/// thread spawn per call.
///
/// A panic inside `f` is propagated as a panic here, matching
/// [`block_on_worker`]'s "tokio worker panicked" convention — a panic
/// in the pass is a bug, not a recoverable condition.
/// ## Panic safety
///
/// A panic inside `f` is CAUGHT (`std::thread::join` captures it) and mapped to
/// an `io::Error`, rather than re-raised: the pre-fix
/// `.expect("big-stack FFI thread panicked")` would have unwound across the
/// `extern "C"` caller and aborted the host on the `unwind` build (`Cargo.toml`
/// policy). The lone caller already threads the returned `io::Result` into its
/// `PlatformWalletFFIResult`. On the iOS `abort` profiles this is inert (the
/// process aborts at the panic site), as documented on [`block_on_worker`].
pub(crate) fn run_on_big_stack_thread<T: Send>(f: impl FnOnce() -> T + Send) -> std::io::Result<T> {
std::thread::scope(|scope| {
let handle = std::thread::Builder::new()
.name("pw-ffi-bigstack".into())
.stack_size(WORKER_STACK_BYTES)
.spawn_scoped(scope, f)?;
Ok(handle.join().expect("big-stack FFI thread panicked"))
match handle.join() {
Ok(value) => Ok(value),
Err(panic_payload) => {
let reason = panic_payload_message(panic_payload.as_ref());
tracing::error!(
target: "platform_wallet_ffi",
"big-stack FFI thread panicked: {reason}"
);
Err(std::io::Error::other(format!(
"big-stack FFI thread panicked: {reason}"
)))
}
}
})
}

Expand Down Expand Up @@ -122,6 +265,48 @@ mod tests {
let out = run_on_big_stack_thread(|| recurse(1_000)).expect("spawn should succeed");
assert!(out > 0);
}

// --- Panic safety (the abort-hazard fix) -------------------------------
//
// Gated to the `unwind` config: on an `abort`-configured build the panic
// aborts the process at the panic site (documented, accepted iOS behavior),
// so there is no recoverable outcome to assert. Under the normal test
// profile (`unwind`) these prove a panicking future/closure returns the
// typed error instead of aborting the runner.

#[cfg(panic = "unwind")]
#[test]
fn block_on_worker_recovers_panicking_future_as_typed_error() {
let out: Result<u32, platform_wallet::PlatformWalletError> =
block_on_worker(async { panic!("boom in worker") });
match out {
Err(platform_wallet::PlatformWalletError::InternalPanic(msg)) => {
assert!(
msg.contains("did not complete") || msg.contains("boom in worker"),
"unexpected recovered message: {msg}"
);
}
other => panic!("expected recovered InternalPanic, got {other:?}"),
}
}

#[cfg(panic = "unwind")]
#[test]
fn block_on_worker_success_path_is_unchanged() {
let out: Result<u32, platform_wallet::PlatformWalletError> =
block_on_worker(async { Ok(7) });
assert!(matches!(out, Ok(7)));
}

#[cfg(panic = "unwind")]
#[test]
fn run_on_big_stack_thread_maps_panic_to_io_error() {
let result: std::io::Result<()> =
run_on_big_stack_thread(|| panic!("boom on big stack"));
let err = result.expect_err("a panicking closure must map to Err, not abort");
assert_eq!(err.kind(), std::io::ErrorKind::Other);
assert!(err.to_string().contains("big-stack FFI thread panicked"));
}
}

#[cfg(feature = "tokio-metrics")]
Expand Down
48 changes: 39 additions & 9 deletions packages/rs-platform-wallet-ffi/src/shielded_send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,15 +597,19 @@ fn map_spend_result(
format!("{operation} failed: {e}"),
),
// The cached Platform Payment-account set no longer covers the
// requested claim plus input-0's fee reserve. Keep this distinct from
// generic wallet-operation failures so hosts can refresh preflight and
// re-confirm a smaller amount instead of retrying unchanged.
Err(e @ PlatformWalletError::PlatformShieldCapacityExceeded { .. }) => {
PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance,
format!("{operation} failed: {e}"),
)
}
// requested claim plus input-0's fee reserve (account-wide), or a live
// per-input hard balance check found one input short (a stale-snapshot
// race). Both share this code — the host's corrective action is the same
// (refresh preflight, retry) — and both stay distinct from generic
// wallet-operation failures so a host never retries the stale amount
// unchanged. The per-input variant's message names the short address.
Err(
e @ (PlatformWalletError::PlatformShieldCapacityExceeded { .. }
| PlatformWalletError::PlatformShieldInputShortfall { .. }),
) => PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance,
format!("{operation} failed: {e}"),
),
Err(e) => PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
format!("{operation} failed: {e}"),
Expand Down Expand Up @@ -1852,6 +1856,32 @@ mod tests {
);
}

#[test]
fn map_spend_result_maps_per_input_shortfall_to_same_code_with_address() {
// The per-input shortfall (a live stale-snapshot race) must ride the
// same code as the account-capacity variant — not regress to the
// generic ErrorWalletOperation — and keep the offending address in the
// message so a host never misreads the single input's balance as the
// account maximum.
let result = map_spend_result(
Err(PlatformWalletError::PlatformShieldInputShortfall {
address: "yShieldInputAddrExample".to_string(),
available: 3_623_849_220,
required: 3_623_849_221,
}),
"shielded shield",
);

assert_eq!(
result.code,
PlatformWalletFFIResultCode::ErrorShieldedInsufficientBalance
);
let message = message_of(&result);
assert!(message.contains("yShieldInputAddrExample"), "message: {message}");
assert!(message.contains("3623849220"));
assert!(message.contains("3623849221"));
}

#[test]
fn map_asset_lock_funding_result_preserves_already_consumed_code_only() {
let out_point = dashcore::OutPoint {
Expand Down
31 changes: 31 additions & 0 deletions packages/rs-platform-wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,26 @@ pub enum PlatformWalletError {
#[error("Platform shield capacity exceeded: available {available}, required {required}")]
PlatformShieldCapacityExceeded { available: u64, required: u64 },

/// A shield's pre-broadcast per-input hard balance check found ONE input
/// address short: its live on-chain balance dropped below what the cached
/// planner snapshot assumed (a stale-snapshot race), so the fetched claim
/// cannot be funded. STRICTLY per-input — `available`/`required` are that
/// single address's live figures, NOT an account-capacity total. Distinct
/// from [`PlatformShieldCapacityExceeded`](Self::PlatformShieldCapacityExceeded)
/// (an account-wide deterministic-selection limit) precisely so a host never
/// misreads this per-address `available` as the account maximum (it would
/// understate capacity by up to the versioned max input count). The
/// offending `address` (bech32m) is preserved in the message rather than
/// dropped. Nothing was built or broadcast; refresh preflight and retry.
#[error(
"Shield input address {address} is short: has {available}, requires at least {required}"
)]
PlatformShieldInputShortfall {
address: String,
available: u64,
required: u64,
},

#[error("Shielded build error: {0}")]
ShieldedBuildError(String),

Expand Down Expand Up @@ -579,6 +599,17 @@ pub enum PlatformWalletError {

#[error("Shielded sub-wallet not bound: call bind_shielded first")]
ShieldedNotBound,

/// An internal async task or big-stack worker panicked and was RECOVERED at
/// the FFI runtime boundary ([`block_on_worker`](crate) /
/// `run_on_big_stack_thread`) instead of being re-raised across the
/// `extern "C"` boundary, which would abort the host process on the
/// `unwind` (Android/host) build. Always an internal bug; the payload is the
/// panic message. Surfaced to the FFI as the generic `ErrorUnknown` code
/// (an unexpected internal failure) with the panic text in the message.
/// Not retryable as-is.
#[error("Internal task panicked (recovered): {0}")]
InternalPanic(String),
}

/// Check whether an SDK error indicates that an InstantSend lock proof was
Expand Down
Loading
Loading