Skip to content

fix(platform-wallet): spv error propagation #3810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ZocoLini wants to merge 1 commit into
base: v3.1-dev
Choose a base branch
from
+39 −30
Open

fix(platform-wallet): spv error propagation #3810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions packages/rs-platform-wallet-ffi/src/spv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,19 @@ pub unsafe extern "C" fn platform_wallet_manager_spv_start(
config.devnet = Some(devnet);
}

let _guard = runtime().enter();
manager.spv_arc().spawn_in_background(config);
let start_result = runtime().block_on(manager.spv().start(config));

if start_result.is_ok() {
let _guard = runtime().enter();
manager.spv_arc().spawn_run_loop();
}
Comment on lines +361 to +366

@thepastaclaw thepastaclaw Jun 12, 2026

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: Poll spv().start(config) on the worker runtime, not the FFI caller thread

runtime().block_on(manager.spv().start(config)) polls the new (substantial) startup future — PeerNetworkManager::new, DiskStorageManager::new, DashSpvClient::new — directly on the foreign caller's thread. packages/rs-platform-wallet-ffi/src/runtime.rs explicitly documents that iOS dispatch/concurrency threads have ~512 KB stacks and that async FFI work should use block_on_worker, which parks the caller on a oneshot and runs the future on the 8 MB-stack worker. The rest of this crate consistently follows that pattern. Previously this work happened inside tokio::spawn on the runtime; the PR's new ordering reintroduces the exact stack-sensitive pattern the runtime module was built to avoid. Switch to block_on_worker using the Arc<SpvRuntime> so the future is Send + 'static.

Suggested change
let start_result = runtime().block_on(manager.spv().start(config));
if start_result.is_ok() {
let _guard = runtime().enter();
manager.spv_arc().spawn_run_loop();
}
let spv = manager.spv_arc();
let start_result = crate::runtime::block_on_worker(async move { spv.start(config).await });
if start_result.is_ok() {
let _guard = runtime().enter();
manager.spv_arc().spawn_run_loop();
}
start_result

source: ['codex']


start_result
});
unwrap_option_or_return!(option);

let start_result = unwrap_option_or_return!(option);
unwrap_result_or_return!(start_result);

PlatformWalletFFIResult::ok()
}

Expand Down
3 changes: 0 additions & 3 deletions packages/rs-platform-wallet/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ pub enum PlatformWalletError {
#[error("No wallets configured — add a wallet before starting SPV")]
NoWalletsConfigured,

#[error("SPV client is not running")]
SpvNotRunning,

#[error("SPV error: {0}")]
SpvError(String),

Expand Down
2 changes: 1 addition & 1 deletion packages/rs-platform-wallet/src/manager/accessors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {
}

/// Clone the `Arc<SpvRuntime>` so callers (e.g. FFI) can invoke
/// [`SpvRuntime::spawn_in_background`] which takes `&Arc<Self>`.
/// [`SpvRuntime::spawn_run_loop`] which takes `&Arc<Self>`.
pub fn spv_arc(&self) -> Arc<SpvRuntime> {
Arc::clone(&self.spv_manager)
}
Expand Down
49 changes: 26 additions & 23 deletions packages/rs-platform-wallet/src/spv/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ impl SpvRuntime {
tx: &Transaction,
) -> Result<(), PlatformWalletError> {
let client_guard = self.client.read().await;
let client = client_guard
.as_ref()
.ok_or(PlatformWalletError::SpvNotRunning)?;
let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError(
"SPV Client not started".to_string(),
))?;

client
.broadcast_transaction(tx)
Expand All @@ -117,9 +117,9 @@ impl SpvRuntime {
height: u32,
) -> Result<[u8; 48], PlatformWalletError> {
let client_guard = self.client.read().await;
let client = client_guard
.as_ref()
.ok_or(PlatformWalletError::SpvNotRunning)?;
let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError(
"SPV Client not started".to_string(),
))?;

let llmq_type = LLMQType::from(quorum_type as u8);
let qh = QuorumHash::from_byte_array(quorum_hash).reverse();
Expand All @@ -132,16 +132,16 @@ impl SpvRuntime {
Ok(*quorum.quorum_entry.quorum_public_key.as_ref())
}

/// Run the SPV sync loop until calling [`stop`]. This blocks the current thread.
pub async fn run(&self, config: ClientConfig) -> Result<(), PlatformWalletError> {
tracing::info!("SpvRuntime::run() starting client...");
self.start(config).await?;
tracing::info!("SpvRuntime::run() client started, entering sync loop");

/// Drive the sync loop of an already-[`start`]ed client until [`stop`]
/// is called.
/// [`spawn_run_loop`](Self::spawn_run_loop).
async fn run(&self) -> Result<(), PlatformWalletError> {
Comment on lines +135 to +138

@thepastaclaw thepastaclaw Jun 12, 2026

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: Malformed rustdoc on run() leaves an orphan link line

The doc comment ends with a standalone /// [spawn_run_loop](Self::spawn_run_loop). line that has no leading sentence — an editing residue from the rename. Rustdoc will render this as an orphan paragraph.

Suggested change
/// Drive the sync loop of an already-[`start`]ed client until [`stop`]
/// is called.
/// [`spawn_run_loop`](Self::spawn_run_loop).
async fn run(&self) -> Result<(), PlatformWalletError> {
/// Drive the sync loop of an already-[`start`]ed client until [`stop`]
/// is called. Typically invoked via
/// [`spawn_run_loop`](Self::spawn_run_loop).
async fn run(&self) -> Result<(), PlatformWalletError> {

source: ['claude']

let client_guard = self.client.read().await;
let client = client_guard
.as_ref()
.ok_or(PlatformWalletError::SpvNotRunning)?
.ok_or(PlatformWalletError::SpvError(
"SPV Client not started".to_string(),
))?
.clone();
drop(client_guard);

Expand Down Expand Up @@ -189,10 +189,11 @@ impl SpvRuntime {
stop_result
}

/// Spawn `run()` on the current tokio runtime and return immediately.
/// Spawn the sync loop of an already-[`start`]ed client on the current
/// tokio runtime and return immediately.
///
/// Call [`stop`] to stop it
pub fn spawn_in_background(self: &Arc<Self>, config: ClientConfig) {
pub fn spawn_run_loop(self: &Arc<Self>) {
{
let existing = self.task.lock().expect("spv task mutex poisoned");
if existing.is_some() {
Expand All @@ -206,8 +207,8 @@ impl SpvRuntime {
let this = Arc::clone(self);

let handle = tokio::spawn(async move {
if let Err(e) = this.run(config).await {
tracing::warn!("SpvRuntime background run exited with error: {}", e);
if let Err(e) = this.run().await {
tracing::warn!("SpvRuntime background run loop exited with error: {}", e);
}
});

Expand Down Expand Up @@ -252,9 +253,10 @@ impl SpvRuntime {
/// The SPV client must be running to perform this operation.
pub async fn clear_storage(&self) -> Result<(), PlatformWalletError> {
let client_guard = self.client.read().await;
let client = client_guard
.as_ref()
.ok_or(PlatformWalletError::SpvNotRunning)?;
let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError(
"SPV Client not started".to_string(),
))?;

client
.clear_storage()
.await
Expand All @@ -266,9 +268,10 @@ impl SpvRuntime {
/// The network cannot be changed on a running client.
pub async fn update_config(&self, config: ClientConfig) -> Result<(), PlatformWalletError> {
let client_guard = self.client.read().await;
let client = client_guard
.as_ref()
.ok_or(PlatformWalletError::SpvNotRunning)?;
let client = client_guard.as_ref().ok_or(PlatformWalletError::SpvError(
"SPV Client not started".to_string(),
))?;

client
.update_config(config)
.await
Expand Down
Loading