coral-xyz / anchor

⚓ Solana Sealevel Framework
https://anchor-lang.com
Apache License 2.0
3.36k stars 1.25k forks source link

feat: Add tokio support to anchor_client `RequestBuilder` #3057

Open cryptopapi997 opened 5 days ago

cryptopapi997 commented 5 days ago

Picking up on https://github.com/coral-xyz/anchor/pull/3006 since we need this for Arcium.

Adds request_threadsafe which creates a threadsafe request for usage in tokio. Fully backwards compatible.

Idk if https://github.com/coral-xyz/anchor/pull/3053 breaks this, so would be good to get this PR merged before that one. Lmk what you think @acheroncrypto

vercel[bot] commented 5 days ago

@cryptopapi997 is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

cryptopapi997 commented 4 days ago

Adapted according to your review:

It's mostly backwards compatible, but since RequestBuilder is public, introducing a new generic can be a breaking change for people who implement their own trait for that type (low probability).

True, although even in this case the fix is relatively easy. It's just a matter of implementing it for RequestBuilder<'a, C, Box<dyn Signer + 'a>> instead of RequestBuilder<'a, C>>.

Wdyt about having only request based on the async feature? This would be more breaking imo, as anyone who uses request with async would have to switch to request_threadsafe which requires you to use an Arc<ThreadSafeSigner> instead of any signer like is currently the case.

Lmk what you think @acheroncrypto

acheroncrypto commented 4 days ago

True, although even in this case the fix is relatively easy. It's just a matter of implementing it for RequestBuilder<'a, C, Box<dyn Signer + 'a>> instead of RequestBuilder<'a, C>>.

All SemVer violations can potentially result in build errors for downstream projects, no matter how easy the fix is e.g. https://github.com/coral-xyz/anchor/issues/3044. It's better to be safe in this case, and consider this a breaking change.

cryptopapi997 commented 4 days ago

All SemVer violations can potentially result in build errors for downstream projects, no matter how easy the fix is e.g. https://github.com/coral-xyz/anchor/issues/3044. It's better to be safe in this case, and consider this a breaking change.

You're right. In this case, replacing request with request_threadsafe in async seems ok.

cryptopapi997 commented 4 days ago

All done, take a look @acheroncrypto