feat: Add Send + Sync supertraits and add Send to futures return by dyn-compatible traits

CHANGELOG.md

@@ -9,6 +9,8 @@
 * Added support for RPC client/server version matching through HTTP ACCEPT header (#912).
 * Added a way to ignore invalid input notes when consuming them in a transaction (#898).
 * Added `NoteUpdate` type to the note update tracker to distinguish between different types of updates (#821).
+* Updated `TonicRpcClient` and `Store` traits to be subtraits of `Send` and `Sync` (#926).
+* Updated `TonicRpcClient` and `Store` trait functions to return futures which are `Send` (#926).
 
 ### Changes
 

crates/rust-client/src/rpc/mod.rs

@@ -42,7 +42,6 @@
 use alloc::{boxed::Box, collections::BTreeSet, string::String, vec::Vec};
 use core::fmt;
 
-use async_trait::async_trait;
 use domain::{
     account::{AccountDetails, AccountProofs},
     note::{NetworkNote, NoteSyncInfo},
@@ -93,8 +92,9 @@ use crate::{
 /// The implementers are responsible for connecting to the Miden node, handling endpoint
 /// requests/responses, and translating responses into domain objects relevant for each of the
 /// endpoints.
-#[async_trait(?Send)]
-pub trait NodeRpcClient {
+#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
+#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
+pub trait NodeRpcClient: Send + Sync {
     /// Given a Proven Transaction, send it to the node for it to be included in a future block
     /// using the `/SubmitProvenTransaction` RPC endpoint.
     async fn submit_proven_transaction(

crates/rust-client/src/rpc/tonic_client/mod.rs

@@ -1,12 +1,10 @@
-#![allow(clippy::await_holding_lock)]
 use alloc::{
     boxed::Box,
     collections::{BTreeMap, BTreeSet},
     string::{String, ToString},
     vec::Vec,
 };
 
-use async_trait::async_trait;
 use miden_objects::{
     Digest,
     account::{Account, AccountCode, AccountDelta, AccountId},
@@ -72,16 +70,18 @@ impl TonicRpcClient {
     /// Takes care of establishing the RPC connection if not connected yet. It ensures that the
     /// `rpc_api` field is initialized and returns a write guard to it.
     async fn ensure_connected(&self) -> Result<ApiClient, RpcError> {
-        let mut client = self.client.write();
-        if client.is_none() {
-            client.replace(ApiClient::new_client(self.endpoint.clone(), self.timeout_ms).await?);
+        if self.client.read().is_none() {
+            let new_client = ApiClient::new_client(self.endpoint.clone(), self.timeout_ms).await?;
+            let mut client = self.client.write();
+            client.replace(new_client);
         }
 
-        Ok(client.as_ref().expect("rpc_api should be initialized").clone())
+        Ok(self.client.read().as_ref().expect("rpc_api should be initialized").clone())
     }
 }
 
-#[async_trait(?Send)]
+#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
+#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
 impl NodeRpcClient for TonicRpcClient {
     async fn submit_proven_transaction(
         &self,
@@ -488,3 +488,34 @@ impl NodeRpcClient for TonicRpcClient {
         Ok(block)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::boxed::Box;
+
+    use super::TonicRpcClient;
+    use crate::rpc::{Endpoint, NodeRpcClient};
+
+    fn assert_send_sync<T: Send + Sync>() {}
+
+    #[test]
+    fn is_send_sync() {
+        assert_send_sync::<TonicRpcClient>();
+        assert_send_sync::<Box<dyn NodeRpcClient>>();
+    }
+
+    // Function that returns a `Send` future from a dynamic trait that must be `Sync`.
+    async fn dyn_trait_send_fut(client: Box<dyn NodeRpcClient>) {
+        // This won't compile if `get_block_header_by_number` doesn't return a `Send+Sync` future.
+        let res = client.get_block_header_by_number(None, false).await;
+        assert!(res.is_ok());
+    }
+
+    #[tokio::test]
+    async fn future_is_send() {
+        let endpoint = &Endpoint::devnet();
+        let client = TonicRpcClient::new(endpoint, 10000);
+        let client: Box<TonicRpcClient> = client.into();
+        tokio::task::spawn(async move { dyn_trait_send_fut(client).await });
+    }
+}

crates/rust-client/src/store/mod.rs

@@ -27,7 +27,6 @@ use alloc::{
 };
 use core::fmt::Debug;
 
-use async_trait::async_trait;
 use miden_objects::{
     Digest, Word,
     account::{Account, AccountCode, AccountHeader, AccountId},
@@ -83,7 +82,8 @@ pub use note_record::{
 /// Because the [`Store`]'s ownership is shared between the executor and the client, interior
 /// mutability is expected to be implemented, which is why all methods receive `&self` and
 /// not `&mut self`.
-#[async_trait(?Send)]
+#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
+#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
 pub trait Store: Send + Sync {
     /// Returns the current timestamp tracked by the store, measured in non-leap seconds since
     /// Unix epoch. If the store implementation is incapable of tracking time, it should return

crates/rust-client/src/store/sqlite_store/mod.rs

@@ -100,7 +100,7 @@ impl SqliteStore {
 //
 // To simplify, all implementations rely on inner SqliteStore functions that map 1:1 by name
 // This way, the actual implementations are grouped by entity types in their own sub-modules
-#[async_trait(?Send)]
+#[async_trait]
 impl Store for SqliteStore {
     fn get_current_timestamp(&self) -> Option<u64> {
         let now = chrono::Utc::now();
@@ -358,8 +358,32 @@ pub fn u64_to_value(v: u64) -> Value {
 
 #[cfg(test)]
 pub mod tests {
+    use std::boxed::Box;
+
     use super::SqliteStore;
-    use crate::tests::create_test_store_path;
+    use crate::{store::Store, tests::create_test_store_path};
+
+    fn assert_send_sync<T: Send + Sync>() {}
+
+    #[test]
+    fn is_send_sync() {
+        assert_send_sync::<SqliteStore>();
+        assert_send_sync::<Box<dyn Store>>();
+    }
+
+    // Function that returns a `Send` future from a dynamic trait that must be `Sync`.
+    async fn dyn_trait_send_fut(store: Box<dyn Store>) {
+        // This wouldn't compile if `get_tracked_block_headers` doesn't return a `Send` future.
+        let res = store.get_tracked_block_headers().await;
+        assert!(res.is_ok());
+    }
+
+    #[tokio::test]
+    async fn future_is_send() {
+        let client = SqliteStore::new(create_test_store_path()).await.unwrap();
+        let client: Box<SqliteStore> = client.into();
+        tokio::task::spawn(async move { dyn_trait_send_fut(client).await });
+    }
 
     pub(crate) async fn create_test_store() -> SqliteStore {
         SqliteStore::new(create_test_store_path()).await.unwrap()

crates/rust-client/src/test_utils/mock.rs

@@ -1,6 +1,5 @@
 use alloc::{collections::BTreeSet, sync::Arc, vec::Vec};
 
-use async_trait::async_trait;
 use miden_lib::transaction::TransactionKernel;
 use miden_objects::{
     Digest, Felt,
@@ -210,7 +209,8 @@ impl MockRpcApi {
     }
 }
 use alloc::boxed::Box;
-#[async_trait(?Send)]
+#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
+#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
 impl NodeRpcClient for MockRpcApi {
     async fn sync_notes(
         &self,