Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 4 additions & 4 deletions packages/rs-platform-wallet-ffi/src/asset_lock/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::error::*;
use crate::handle::*;
use crate::runtime::runtime;
use crate::{check_ptr, unwrap_option_or_return};
use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return};

/// C-compatible tracked asset lock entry.
#[repr(C)]
Expand Down Expand Up @@ -54,7 +54,7 @@ pub unsafe extern "C" fn asset_lock_manager_list_tracked_locks(
let option = ASSET_LOCK_MANAGER_STORAGE.with_item(handle, |manager| {
use platform_wallet::AssetLockStatus;

let locks = runtime().block_on(manager.list_tracked_locks());
let locks = runtime().try_block_on(manager.list_tracked_locks())?;
let entries: Vec<TrackedAssetLockFFI> = locks
.iter()
.map(|lock| {
Expand Down Expand Up @@ -86,9 +86,9 @@ pub unsafe extern "C" fn asset_lock_manager_list_tracked_locks(
}
})
.collect();
entries
Ok::<_, platform_wallet::PlatformWalletError>(entries)
});
let entries = unwrap_option_or_return!(option);
let entries = unwrap_result_or_return!(unwrap_option_or_return!(option));

*out_count = entries.len();
if entries.is_empty() {
Expand Down
27 changes: 16 additions & 11 deletions packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ pub unsafe extern "C" fn core_wallet_broadcast_signed_transaction(
// exclusive side, so it cannot interleave between the check and the send.
// Scoped per generation, so this send — up to the broadcaster's timeout —
// blocks only THIS wallet's teardown, never an unrelated wallet's.
let (_lifecycle, wallet_is_live) = runtime().block_on(async {
let (_lifecycle, wallet_is_live) = unwrap_result_or_return!(runtime().try_block_on(async {
let gate = wallet.generation_payment_guard().await;
let live = wallet.is_current_generation().await;
(gate, live)
});
}));
if !wallet_is_live {
runtime().block_on(finalized.wallet.abandon_transaction(&finalized.transaction));
return PlatformWalletFFIResult::err(
Expand Down Expand Up @@ -264,6 +264,7 @@ mod tests {

fn finalize(core: &TestCore, signer: &WalletSigner, tag: u8) -> SignedCoreTransaction {
runtime()
.raw()
.block_on(core.finalize_transaction(
TransactionBuilder::new().add_output(
&Address::dummy(Network::Testnet, usize::from(tag)),
Expand All @@ -285,13 +286,14 @@ mod tests {

fn assert_released(core: &TestCore, signer: &WalletSigner, tag: u8) {
let retry = finalize(core, signer, tag);
runtime().block_on(core.abandon_transaction(&retry));
runtime().raw().block_on(core.abandon_transaction(&retry));
}

#[test]
fn double_free_is_safe_and_releases_reservation() {
let (core, signer) =
runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let (core, signer) = runtime()
.raw()
.block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let transaction_handle = insert(&core, finalize(&core, &signer, 40));

core_wallet_signed_transaction_free(transaction_handle);
Expand All @@ -302,8 +304,9 @@ mod tests {

#[test]
fn invalid_or_wrong_wallet_consumes_and_releases() {
let (origin, origin_signer) =
runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let (origin, origin_signer) = runtime()
.raw()
.block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let invalid_transaction = insert(&origin, finalize(&origin, &origin_signer, 42));
let invalid =
unsafe { core_wallet_abandon_signed_transaction(u64::MAX, invalid_transaction) };
Expand All @@ -313,8 +316,9 @@ mod tests {
);
assert_released(&origin, &origin_signer, 43);

let (other, _) =
runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let (other, _) = runtime()
.raw()
.block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let other_handle = CORE_WALLET_STORAGE.insert(other);
let wrong_transaction = insert(&origin, finalize(&origin, &origin_signer, 44));
let wrong =
Expand All @@ -329,8 +333,9 @@ mod tests {

#[test]
fn abandon_then_free_or_broadcast_cannot_reconsume_handle() {
let (core, signer) =
runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let (core, signer) = runtime()
.raw()
.block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let core_handle = CORE_WALLET_STORAGE.insert(core.clone());
let transaction_handle = insert(&core, finalize(&core, &signer, 46));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::error::*;
use crate::handle::{Handle, CORE_WALLET_STORAGE};
use crate::runtime::runtime;
use crate::{check_ptr, unwrap_option_or_return};
use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return};
use once_cell::sync::Lazy;
use platform_wallet::broadcaster::SpvBroadcaster;
use platform_wallet::{ReservationToken, SignedPaymentError, SignedPaymentRegistry};
Expand Down Expand Up @@ -86,8 +86,17 @@ pub unsafe extern "C" fn core_wallet_signed_payment_broadcast(

let core = unwrap_option_or_return!(CORE_WALLET_STORAGE.with_item(core_handle, |w| w.clone()));

let result =
runtime().block_on(SIGNED_PAYMENT_REGISTRY.broadcast(ReservationToken::from(token), &core));
// `try_block_on`, deliberately NOT a `FromCaughtPanicError` impl on
// `SignedPaymentError`: its only generic-ish variant is `Broadcast(..)`,
// whose payload carries the typed retry semantics of a REAL broadcast
// outcome. A panic must not be dressed up as one — it reaches the host as
// the generic ErrorWalletOperation with the panic text instead.
let result = match runtime()
.try_block_on(SIGNED_PAYMENT_REGISTRY.broadcast(ReservationToken::from(token), &core))
{
Ok(result) => result,
Err(error) => return error.into(),
};

match result {
Ok(txid) => {
Expand Down Expand Up @@ -144,6 +153,10 @@ pub unsafe extern "C" fn core_wallet_signed_payment_broadcast(
/// Always safe to call; `token` is a plain value.
#[no_mangle]
pub unsafe extern "C" fn core_wallet_signed_payment_release(token: u64) -> PlatformWalletFFIResult {
runtime().block_on(SIGNED_PAYMENT_REGISTRY.release(ReservationToken::from(token)));
// `try_block_on`: releasing IS this entry point's job, so a panic must
// not come back as a success the host records as "reservation released".
unwrap_result_or_return!(
runtime().try_block_on(SIGNED_PAYMENT_REGISTRY.release(ReservationToken::from(token)))
);
PlatformWalletFFIResult::ok()
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ pub unsafe extern "C" fn core_wallet_tx_builder_finalize(
// the signer await, never around it: holding it across an open signing prompt
// would stall this wallet's teardown for as long as the user takes, and the
// check makes that unnecessary.
let (_lifecycle, wallet_is_live) = runtime().block_on(async {
let (_lifecycle, wallet_is_live) = unwrap_result_or_return!(runtime().try_block_on(async {
let gate = wallet.core().generation_payment_guard().await;
let live = wallet.core().is_current_generation().await;
(gate, live)
});
}));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

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: Release the reservation before returning a lifecycle-check failure

These newly fallible lifecycle checks run after finalization has acquired a transaction reservation. If try_block_on catches a panic, unwrap_result_or_return! consumes the error path without publishing a transaction handle and without calling abandon_transaction, leaving the host no token or handle with which to release the reservation. Handle the outer Err explicitly and make a best-effort abandonment before returning the original panic error. The same ownership gap exists at transaction_builder.rs:302-306 and core_wallet/broadcast.rs:79-83, using finalized and finalized.transaction respectively.

source: ['coderabbit']

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed and fixed in 84f469e at all three sites you identified (transaction_builder.rs:168-172 and :302-306 with finalized, broadcast.rs:79-83 with finalized.transaction).

Rather than repeat the compensation three times, the release now lives next to the acquisition that can fail: core_wallet/lifecycle.rs::lifecycle_gate_or_release takes the gate, reads liveness, and on a guarded failure abandons the finalized transaction before returning the original error. It takes gate_on and release_on separately because the broadcast path gates on the caller's handle but must act through the transaction's own originating wallet. The abandon stays on the swallowing block_on — this path is already returning an error of its own and a second failure must not mask the first — and the release is generation-bound, so it remains a logged no-op on a genuine removal and correctly declines to touch a re-created generation's inputs.

Two tests. a_lifecycle_gate_panic_releases_the_reservation_and_reports_the_panic drives the real exported core_wallet_broadcast_signed_transaction: it asserts code 6 with the marker at position 0, that no txid is published, that the handle stays consumed — and that the reservation came back, by finalizing a second transaction on the same account, which can only fund if the first build's inputs were released. lifecycle_gate_helper_releases_before_returning_the_failure pins the same guarantee at the shared seam for the two finalize entry points.

One thing worth calling out: the panic is injected through a #[cfg(test)] thread-local hook. The obvious alternative — driving the entry point from inside a runtime context, as the existing abandon test does — makes the compensating abandon_transaction panic as well, so it could never show the reservation returning. The hook makes only the gate acquisition fail, on the real code path.

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.

Resolved in 84f469eRelease the reservation before returning a lifecycle-check failure no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

if !wallet_is_live {
// No handle was published, so nothing would ever release this build's
// reservation. Reconcile it here: the release is generation-bound, so on
Expand Down Expand Up @@ -299,11 +299,11 @@ pub unsafe extern "C" fn core_wallet_signed_payment_finalize(
// Deliberately acquired AFTER the signer await rather than around it: holding
// it across an open signing prompt would stall this wallet's teardown for as
// long as the user takes, and the check below makes that unnecessary.
let (_lifecycle, wallet_is_live) = runtime().block_on(async {
let (_lifecycle, wallet_is_live) = unwrap_result_or_return!(runtime().try_block_on(async {
let gate = wallet.core().generation_payment_guard().await;
let live = wallet.core().is_current_generation().await;
(gate, live)
});
}));
if !wallet_is_live {
// Nothing was registered, so no token would ever release this build's
// reservation. Reconcile it here: the release is generation-bound, so on
Expand Down
16 changes: 9 additions & 7 deletions packages/rs-platform-wallet-ffi/src/dashpay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::contact_request::CONTACT_REQUEST_STORAGE;
use crate::error::*;
use crate::established_contact::ESTABLISHED_CONTACT_STORAGE;
use crate::handle::*;
use crate::runtime::block_on_worker;
use crate::runtime::{block_on_worker, try_block_on_worker};
use crate::types::*;
use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return};

Expand Down Expand Up @@ -878,7 +878,7 @@ pub unsafe extern "C" fn platform_wallet_drain_pending_contact_crypto(
network,
)
};
block_on_worker(async move {
try_block_on_worker(async move {
let drained = identity
.dashpay()
.drain_pending_contact_crypto(&provider)
Expand All @@ -897,7 +897,7 @@ pub unsafe extern "C" fn platform_wallet_drain_pending_contact_crypto(
drained + accepted
})
});
let total = unwrap_option_or_return!(option);
let total = unwrap_result_or_return!(unwrap_option_or_return!(option));
unsafe {
*out_drained = total as u32;
}
Expand Down Expand Up @@ -927,9 +927,9 @@ pub unsafe extern "C" fn platform_wallet_pending_contact_crypto_count(

let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| {
let identity = wallet.identity().clone();
block_on_worker(async move { identity.dashpay().pending_contact_crypto_count().await })
try_block_on_worker(async move { identity.dashpay().pending_contact_crypto_count().await })
});
let count = unwrap_option_or_return!(option);
let count = unwrap_result_or_return!(unwrap_option_or_return!(option));
unsafe {
*out_count = count as u32;
}
Expand Down Expand Up @@ -1201,9 +1201,11 @@ pub unsafe extern "C" fn platform_wallet_drainable_contact_crypto_count(

let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| {
let identity = wallet.identity().clone();
block_on_worker(async move { identity.dashpay().drainable_contact_crypto_count().await })
try_block_on_worker(
async move { identity.dashpay().drainable_contact_crypto_count().await },
)
});
let count = unwrap_option_or_return!(option);
let count = unwrap_result_or_return!(unwrap_option_or_return!(option));
unsafe {
*out_count = count as u32;
}
Expand Down
8 changes: 4 additions & 4 deletions packages/rs-platform-wallet-ffi/src/dashpay_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use std::time::Duration;

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

/// Start the recurring DashPay sync loop in the background. Idempotent
/// — calling while already running is a no-op.
Expand Down Expand Up @@ -170,9 +170,9 @@ pub unsafe extern "C" fn platform_wallet_manager_dashpay_sync_sync_now(
// the ~512 KB stack of the iOS calling thread (SIGBUS observed
// on-device 2026-06-12). The worker dispatch moves the compute
// onto the runtime's 8 MB-stack threads (see runtime.rs).
block_on_worker(async move { mgr.sync_now().await })
try_block_on_worker(async move { mgr.sync_now().await })
});
let summary = unwrap_option_or_return!(option);
let summary = unwrap_result_or_return!(unwrap_option_or_return!(option));

if !out_success_count.is_null() {
*out_success_count = summary.success_count();
Expand Down
8 changes: 4 additions & 4 deletions packages/rs-platform-wallet-ffi/src/dpns_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use std::time::Duration;

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

/// Start the recurring DPNS marketplace sync loop in the background.
/// Idempotent — calling while already running is a no-op.
Expand Down Expand Up @@ -164,9 +164,9 @@ pub unsafe extern "C" fn platform_wallet_manager_dpns_sync_sync_now(
// GroveDB document-query proofs whose recursion blows the ~512 KB
// stack of the iOS calling thread. The worker dispatch moves the
// compute onto the runtime's 8 MB-stack threads (see runtime.rs).
block_on_worker(async move { mgr.sync_now().await })
try_block_on_worker(async move { mgr.sync_now().await })
});
let summary = unwrap_option_or_return!(option);
let summary = unwrap_result_or_return!(unwrap_option_or_return!(option));

if !out_success_count.is_null() {
*out_success_count = summary.success_count();
Expand Down
11 changes: 11 additions & 0 deletions packages/rs-platform-wallet-ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,17 @@ impl From<PlatformWalletError> for PlatformWalletFFIResult {
// rides `NotFound` rather than spending a fifth marketplace
// code hosts would handle identically.
PlatformWalletError::DpnsNameNotFound { .. } => PlatformWalletFFIResultCode::NotFound,
// A panic caught below an entry point by `crate::panic_guard` and
// converted into a value instead of being allowed to abort the
// host. It rides the crate's GENERIC code deliberately: a panic
// proves nothing about whether the operation reached the network,
// so it must not borrow any of the codes above that carry a
// retry/outcome contract. The message keeps
// `panic_guard::FFI_PANIC_PREFIX` at position 0, so a host (or a
// log grep) can still separate it from an ordinary code-6 failure.
PlatformWalletError::InternalPanic(..) => {
PlatformWalletFFIResultCode::ErrorWalletOperation
}
// NOTE: `MessageSigningFailed` is deliberately NOT matched, so it
// falls to the `ErrorUnknown` catch-all below. Its causes are
// internal invariant breaks (a public key that does not own the
Expand Down
33 changes: 17 additions & 16 deletions packages/rs-platform-wallet-ffi/src/identity_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use platform_wallet::{IdentityTokenSyncInfo, IdentityTokenSyncState};

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

/// Flattened per-(identity, token) row mirroring
/// [`IdentityTokenSyncInfo`].
Expand Down Expand Up @@ -136,9 +136,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_last_sync_unix_se

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
let mgr = manager.identity_sync_arc();
runtime().block_on(async move { mgr.last_sync_unix_for_identity(&identity_id).await })
runtime().try_block_on(async move { mgr.last_sync_unix_for_identity(&identity_id).await })
});
let value = unwrap_option_or_return!(option);
let value = unwrap_result_or_return!(unwrap_option_or_return!(option));
*out_last_sync_unix = value.unwrap_or(0);
PlatformWalletFFIResult::ok()
}
Expand Down Expand Up @@ -175,9 +175,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_sync_now(
// stack of the host's dispatch/concurrency calling thread
// (same SIGBUS as the shielded/dashpay Sync Now buttons).
let mgr = manager.identity_sync_arc();
block_on_worker(async move { mgr.sync_now().await });
try_block_on_worker(async move { mgr.sync_now().await })
});
unwrap_option_or_return!(option);
unwrap_result_or_return!(unwrap_option_or_return!(option));
PlatformWalletFFIResult::ok()
}

Expand Down Expand Up @@ -214,9 +214,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_state_for_identit

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
let mgr = manager.identity_sync_arc();
runtime().block_on(async move { mgr.state_for_identity(&identity_id).await })
runtime().try_block_on(async move { mgr.state_for_identity(&identity_id).await })
});
let row = unwrap_option_or_return!(option);
let row = unwrap_result_or_return!(unwrap_option_or_return!(option));
match row {
Some(state) => {
let rows: Vec<IdentityTokenSyncInfoFFI> = state
Expand Down Expand Up @@ -263,9 +263,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_state_all(

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
let mgr = manager.identity_sync_arc();
runtime().block_on(async move { mgr.all_state().await })
runtime().try_block_on(async move { mgr.all_state().await })
});
let snapshot = unwrap_option_or_return!(option);
let snapshot = unwrap_result_or_return!(unwrap_option_or_return!(option));
let mut rows: Vec<IdentityTokenSyncInfoFFI> = Vec::new();
for state in snapshot.values() {
for info in &state.tokens {
Expand Down Expand Up @@ -347,9 +347,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_register_identity

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
let mgr = manager.identity_sync_arc();
runtime().block_on(async move { mgr.register_identity(identity_id, token_ids).await });
runtime().try_block_on(async move { mgr.register_identity(identity_id, token_ids).await })
});
unwrap_option_or_return!(option);
unwrap_result_or_return!(unwrap_option_or_return!(option));
PlatformWalletFFIResult::ok()
}

Expand All @@ -369,9 +369,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_unregister_identi

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
let mgr = manager.identity_sync_arc();
runtime().block_on(async move { mgr.unregister_identity(&identity_id).await });
runtime().try_block_on(async move { mgr.unregister_identity(&identity_id).await })
});
unwrap_option_or_return!(option);
unwrap_result_or_return!(unwrap_option_or_return!(option));
PlatformWalletFFIResult::ok()
}

Expand Down Expand Up @@ -405,8 +405,9 @@ pub unsafe extern "C" fn platform_wallet_manager_identity_sync_update_watched_to

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
let mgr = manager.identity_sync_arc();
runtime().block_on(async move { mgr.update_watched_tokens(identity_id, token_ids).await });
runtime()
.try_block_on(async move { mgr.update_watched_tokens(identity_id, token_ids).await })
});
unwrap_option_or_return!(option);
unwrap_result_or_return!(unwrap_option_or_return!(option));
PlatformWalletFFIResult::ok()
}
1 change: 1 addition & 0 deletions packages/rs-platform-wallet-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub mod manager;
pub mod manager_diagnostics;
pub mod memory_explorer;
pub mod mnemonic_words;
mod panic_guard;
pub mod persistence;
pub mod platform_address_sync;
pub mod platform_address_types;
Expand Down
Loading
Loading