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

Enable async and block features without duplicating code #1375

Closed
notmandatory opened this issue Mar 15, 2024 · 3 comments
Closed

Enable async and block features without duplicating code #1375

notmandatory opened this issue Mar 15, 2024 · 3 comments
Labels
module-blockchain new feature New feature or request
Milestone

Comments

@notmandatory
Copy link
Member

Describe the enhancement

It would be nice to find or create a macro to generate async and blocking versions from the same source code. The solution should allow both implementation to coexist, build and run if feature flags for both are enabled. It should also be possible to test both versions with the --all-features flag.

Use case

In the bdk_esplora package code is duplicated in the async_ext and blocking_ext modules.

Additional context

In the pre-1.0.0 code we used a maybe_async! macro created by @afilini but it can only create async OR block code, and for blocking it uses tokio::Runtime::block_on() so still requires the tokio dependency.

This issue is inspired by a discussion with @storopoli and the article he shared during a PR code review: "The bane of my existence: Supporting both async and sync code in Rust".

The synca crate is relatively new but looks like it provides a good solution to this problem.

@notmandatory notmandatory added the new feature New feature or request label Mar 15, 2024
@notmandatory notmandatory added this to the 1.0.0-alpha milestone Mar 15, 2024
@notmandatory notmandatory added this to BDK Mar 15, 2024
@notmandatory notmandatory added module-blockchain api A breaking API change and removed api A breaking API change labels Mar 16, 2024
@storopoli
Copy link
Contributor

synca seems really nice and saw some comments in the blogpost was mentioning it.
One thing that concerns me is that the crate, as you've said, brand new.
However it only has 3 dependencies:

  1. proc-macro2
  2. quote
  3. syn

All of these are pretty established and past maturity (^1.0) and also from dtolnay,
so I don't think that synca will give us headaches in the future.

I think we can move this to beta milestone no?
I don't think it will change the API but definitely an internal performance/refactor thing.
If this is for the beta milestone, I might want to take a look into it and see if I can come up with a PR.

@notmandatory
Copy link
Member Author

Thanks for looking more into this. And yes since this shouldn't change any APIs I'll put it in the beta milestone. But if we can't get it done there we can always push it out to a 1.1 release.

@notmandatory notmandatory modified the milestones: 1.0.0-alpha, 1.0.0-beta Mar 16, 2024
@notmandatory notmandatory moved this to Todo in BDK Mar 16, 2024
@LLFourn
Copy link
Contributor

LLFourn commented Mar 17, 2024

The code is not entirely duplicated though :. The sync esplora uses threads while the other one doesn't. Would be interested to see if you can finangle a solution there.

@notmandatory notmandatory closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2024
@github-project-automation github-project-automation bot moved this from Discussion to Done in BDK Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module-blockchain new feature New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants