feat: use dlc-macros for async dlc-manager

[bennyhodl]

Alternate to #235

bdk_macros uses old versions of syn and is no longer maintained so it might be useful to create a dlc_macros crate.

This PR gives an example of a different implementation to support async manager compared to copying functions.

[bennyhodl]

bdk-macros did not quite fit our use case because it compiles to wasm target. I took inspiration from it and updated to latest dependencies with dlc-macros.

It requires a dependency of async_trait but by updating all crates to 2021 then we could remove that dependency.

[bennyhodl]

There are conflicts with the macro expansion and the unit tests, so i made a script to ignore testing async with all features.

[Tibo-lg]

Sorry I need to take some time to review that but don't have it right now. But regarding testing, I don't think it's nice to have to run a script to be able to run unit tests. Also I think it'd be good to have the async feature tested in some way, AFAICT it's not the case?

[bennyhodl]

I can write tests for the async.

I'm running into an issue with cargo that it tries compiling the non-async implementations. I'm not sure how we should get around this. Any ideas?

[Tibo-lg]

I'm running into an issue with cargo that it tries compiling the non-async implementations. I'm not sure how we should get around this. Any ideas?

Do you have more details on this? Like what error do you get?

[bennyhodl]

I think running all features on the workspace it tries to compile the oracle clients with both async and sync even though those flags are provided in the p2pd-oracle-client and mocks.

$ cargo test --all-features

Checking p2pd-oracle-client v0.1.0 (/Users/ben/Development/rust-dlc/p2pd-oracle-client)
error[E0195]: lifetime parameters or bounds on method `get_announcement` do not match the trait declaration
   --> p2pd-oracle-client/src/lib.rs:152:24
    |
152 |     fn get_announcement(&self, event_id: &str) -> Result<OracleAnnouncement, DlcManagerError> {
    |                        ^ lifetimes do not match method in trait

error[E0195]: lifetime parameters or bounds on method `get_attestation` do not match the trait declaration
   --> p2pd-oracle-client/src/lib.rs:159:23
    |
159 |     fn get_attestation(
    |                       ^ lifetimes do not match method in trait

[bennyhodl]

Updated and tests are passing now. I made the integration tests under a module that is under #[cfg(not(feature = "async"))] I did not add tests for the async implementation yet. I have been using the async branches for dlcdevkit.

I'm not sure about using the macro for async. I think the implementation is harder to reason about than the alternative PR.

Still works though ¯_(ツ)_/¯

[bennyhodl]

Although, on second thought this might be a better solution with adding potentially an async blockchain interface.

[Tibo-lg]

Both approaches have pros and cons but I also prefer this one. I just need to find time to review your code, but if you find someone else that could review it as well it would be great.

To merge this I think I also want to have at least some integration tests using async otherwise we don't know whether it's actually working or not.

[bennyhodl]

@Tibo-lg I have spent a few days trying to get the async tests running. I keep having play whack a mole with the testing dependencies.

I have been using this branch quite a bit in dlcdevkit. CI is passing and there is no code change to core functionality.

Can this get merged?

[Tibo-lg]

Sorry but I don't want to merge something that is not tested, because then we don't know first if it's actually working, and more importantly we won't know if it get broken by future changes.

I can try to help you get the tests working though, is there anything you have that is worth sharing or should I just start from scratch?

Also note as mentioned in DM that while I'm ok to merge it for now, I don't think that's the direction I want the library to take in the future, and I think it will be better to push async issues out of the library boundaries to avoid having to deal with the complexity.

[bennyhodl]

This is a commit I had running async tests. 4bf936f

& the corresponding action https://github.com/p2pderivatives/rust-dlc/actions/runs/11942863204

Locally tests were passing but the tests failed with dropping a tokio runtime for blocking operations.

I believe I tracked it down to the Esplora client and the blocking operations with the Blockchain interface.

[Tibo-lg]

I get the same error when running locally. I guess it might be necessary to make the blockchain interface async as well?