Instantiating Miden Client in Backend Causes Route Handlers to Fail

[OgnjenCBS]

Feature description

The problem occurs when instantiation of Miden Client is attempted in a backend server (Axum). The core issue lies in the fact that the SEND and SYNC traits are not implemented for several types in the Miden Client, so any usage of created async functions in Miden SDK for Orderbook will not be able to be awaited on the server side.

1. Handler-Level Instantiation: If I instantiate the client within a handler function, the handler function itself cannot be used as a part of a route due to the lack of SEND and SYNC implementation
2. Application-Level Instantiation: If I instantiate at the application level and make it part of the application state, none of the handlers would work since Miden Client is now a part of the state.

This is an example of the code that will produce an error

pub async fn create_limit_order(
    Path(pair): Path<String>,
    State(controller): State<ExchangeModelController>,
    Json(payload): Json<CreateLimitOrderRequest>,
) {
    let mut client = create_client().await;

    let order_creator = AccountId::from_hex(&payload.order_creator).unwrap();

    // Example of using the client for creating a limit order
    // Uncommenting this block leads to errors
    // match payload.side {
    //     Side::Bid => {
    //         let offered_asset = AccountId::from_hex(&payload.price_acc_id).unwrap();
    //         let requested_asset = AccountId::from_hex(&payload.quantity_acc_id).unwrap();

    //         let lo = limit_order::LimitOrder::create_limit_order(
    //             &mut client,
    //             None,
    //             order_creator,
    //             offered_asset,
    //             payload.price,
    //             requested_asset,
    //             payload.quantity,
    //         )
    //         .await
    //         .unwrap();

    //         lo.submit_limit_order(&mut client).await;
    //     }
    //     Side::Ask => {
    //         let offered_asset = AccountId::from_hex(&payload.quantity_acc_id).unwrap();
    //         let requested_asset = AccountId::from_hex(&payload.price_acc_id).unwrap();

    //         let lo = limit_order::LimitOrder::create_limit_order(
    //             &mut client,
    //             None,
    //             order_creator,
    //             offered_asset,
    //             payload.price,
    //             requested_asset,
    //             payload.quantity,
    //         )
    //         .await
    //         .unwrap();

    //         lo.submit_limit_order(&mut client).await;
    //     }
    // }

    // Update the orderbook
    let mut exchange = controller.get_orderbook_mut(&pair).unwrap();
    let orderbook = exchange.get_mut(&pair).unwrap();

    orderbook.add_limit_order(payload.quantity, payload.side, payload.price);
}

Most of the errors will show in this form

the trait Syncis not implemented for(dyn miden_tx::prover::TransactionProver + 'static), which is required by impl Future<Output = ()>: Send``

the trait Sendis not implemented for(dyn miden_tx::prover::TransactionProver + 'static), which is required by impl Future<Output = ()>: Send``

Environment:

miden-client: version 0.6.0
miden-objects: version 0.6.0

axum: version 0.7.9
tokio: version 1.0

Why is this feature needed?

No response

[phklive]

@Mirko-von-Leipzig @igamigo could you guys give it a look, I know that you have a lot of exp with Rust and multi-threading.

If not you could you re-route to someone that can give a hand, thanks.

[tomyrd]

I've been working on this, there's a version of the Client in the tomyrd-send-sync-client branch (PR #659) that should be Send+Sync (this is the latest version of the client, so there will be some interfaces changes from v0.6) and should fix the mentioned issue.

There might another problem with that version though. Specifically, all the async methods of the client will return Futures that are not Send. This might not seem as a problem, but tokio tasks require that all of the data held is Send. This might or might not be a problem in the original issue, I don't have enough context to determine this.

Even with a full Send + Sync client, we haven't tested enough with the client to know if the behavior would be correct. The current client is not meant to be used concurrently, there are problems that might arise with concurrent use (like race conditions when using the same account at the same time). If the access to the client is controlled this might not be an issue.

A possible workaround in this case is to create new instances of the client that all share the same sqlite database, this means that every client would share the data and be undistinguishable from each other. In this case an external lock could be used to make sure the client is only accessed by one thread at a time.

[bobbinth]

I don't have the full context, but this reminded me of the discussion we had a while back in #358 (comment). Would the SharableClient approach work here?

[phklive]

Written by the Keom team:

Keom<>Miden Asynchronous - Multithreading Problem

Problem exposure

While developing the Keom backend, I encountered a significant issue with the Orderbook SDK that we have created and its integration into the Axum-based API. Specifically, many types within the Miden Client do not implement Send or Sync traits, which creates a roadblock when attempting to await functions in handler routes.

Context

Axum relies on Tokio as the asynchronous runtime. Tokio, by default, uses a multithreaded runtime, requiring that all futures implement the Send trait to ensure they can be safely moved across threads.
Additionally, shared state in Axum often needs to be Sync to allow concurrent access from multiple threads

The Orderbook SDK that we created contains functions and data structures integral to managing orderbook operations. However, many of these types do not implement Send or Sync.

This limitation prevents these types and functions from being used in Axum routes since the handler functions are expected to operate seamlessly in Tokio’s multithreaded environment.

After opening a github issue, core developers proposed different solutions. I have tried implementing those solutions as trying to implement some of the solutions that I thought would work. In the next section, I will explain each and one of them.

Tomyrd-send-sync-client Branch

The issue arises because the new client introduces significant changes, including new methods and removal of old ones. While attempting to refactor the SDK to align with version 0.7, I encountered several challenges due to the lack of examples or documentation for implementing certain features. For instance, the AccountTemplate type no longer exists, which left me unsure about how to create faucets and user accounts under the new implementation.
The updated client branch introduces breaking changes to our existing codebase. While this could potentially be resolved, it would require time and effort, together with back-and-forth communication with the Miden team for clarification and support.

As Tomyrd stated: “This is the latest version of the client, so there will be some interface changes from v0.6”

This turned out to be a bigger issue than expected.

SharableClient

Initially, I assumed that SharableClient was something provided by the Miden Client itself. However, after searching for it, I realized that this is not the case. Upon reading through a related discussion, I discovered that SharableClient needs to be implemented by the developer working on the application.
I attempted to implement this by wrapping the Client in an Arc and a Mutex, which allowed the client to be shared across threads. However, this approach introduced a new issue: The value needs to be locked and unwrapped in a thread where functions from a client are intended to be used. When attempting to use those functions, the process propagated the requirement for Sync and Send implementations from the lowest-level Miden Client types all the way up to my handler function, creating the same cascade of compatibility problems.

Making the whole application single-threaded

The initial idea was to make the entire application run on a single thread, eliminating the need for Send and Sync (Single-threaded Tokio). After introducing the necessary macro change for the main function, the problem still persisted, which was confusing. Why would Send and Sync still be required in a single-threaded environment? This had me raise the question on the Tokio Discord server.

I received two responses. The first suggested using the current-thread flavor of Tokio’s runtime along with LocalSet to remove the need for Send (LocalSet). I attempted to wrap app instantiation in the main function within a LocalSet, but it didn’t give me the results I was hoping for. Honestly, I can’t explain why this approach failed, as it goes beyond my expertise.

The second suggestion was to create custom mpsc channels to handle communication. In this approach, the sender would be given to the handler, and the receiver would be a thread. While this method might promise, implementing would require more time to explore the best approach and understanding a problem to its fundamentals, as this is not a type of problem I frequently encounter

@tomyrd @igamigo @bobbinth