Skip to content
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.

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

Add Asynchronous Support to DataStore Trait Methods #668

Closed
dagarcia7 opened this issue May 6, 2024 · 2 comments · Fixed by #725
Closed

Add Asynchronous Support to DataStore Trait Methods #668

dagarcia7 opened this issue May 6, 2024 · 2 comments · Fixed by #725
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@dagarcia7
Copy link
Contributor

dagarcia7 commented May 6, 2024

Feature description

The current implementation of the DataStore trait in the repository includes two methods (get_transaction_inputs, and get_account_code) that are synchronous.

To enhance the flexibility and usability of the CLI tool that utilizes this trait, particularly in diverse environments like web browsers, it is proposed that asynchronous versions of these methods be added. This would allow the DataStore trait to support both synchronous and asynchronous operations, thereby accommodating a broader range of backend storage solutions and usage contexts such as indexedDB.

Suggested Approach:

  • Add a feature to allow conditional compilation of an DataStore trait with async methods
  • Add the #[async_trait(?Send)] annotation
  • Mark the new methods as async
  • Ensure that anywhere these methods are called within miden-base are marked as async and any async functions called are awaited ("async all the way down").

Why is this feature needed?

The DataStore trait is currently designed for synchronous operation, which aligns well with backend solutions like SQLite3 used in server or desktop environments. However, we need to adapt the existing Rust-based CLI tool to run in a web browser environment, where it interacts with IndexedDB through JavaScript calls for data storage. JavaScript's nature and the IndexedDB API are inherently asynchronous, making the current synchronous DataStore trait methods unsuitable for this context.

Switching to asynchronous methods would not only resolve compatibility issues but also improve performance and responsiveness when the CLI tool is deployed in a browser. This change would enable efficient data operations without blocking the main thread, leading to a better user experience and broader applicability of the tool in asynchronous programming environments.

Additionally, this enhancement would maintain backward compatibility by retaining the existing synchronous methods for scenarios where asynchronous operations are unnecessary or undesirable. This dual approach ensures that the DataStore trait remains flexible and applicable across a variety of use cases.

@dagarcia7 dagarcia7 added the enhancement New feature or request label May 6, 2024
@bobbinth
Copy link
Contributor

bobbinth commented May 7, 2024

One option which could minimize the amount of changes needed is to use something like maybe-async proc macro. A slightly modified version of the macro will soon be merged into Winterfell (see facebook/winterfell#276). Once this is done, we could:

  • Add async feature to the miden-tx crate (turned off by default).

  • Change DataStore trait to look something like:

    #[maybe_async]
    pub trait DataStore {
        async fn get_transaction_inputs(
            &self,
            account_id: AccountId,
            block_ref: u32,
            notes: &[NoteId],
        ) -> Result<TransactionInputs, DataStoreError>;
    
        async fn get_account_code(&self, account_id: AccountId) -> Result<ModuleAst, DataStoreError>;
    }

Then, when the crate is used with async feature enabled, we'd get the async versions of the trait, and we'd get the default version otherwise.

@phklive
Copy link
Contributor

phklive commented Jun 11, 2024

Closed by #725

@phklive phklive closed this as completed Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants