async-signature: move to AFIT #1419
Conversation
`AFIT` is expected in the upcoming rust-1.75 release (scheduled for 2023/12/28). `#[allow(async_fn_in_trait)]` is required until a solution to RPITIT is merged in rust. This put responsability on the implementor of `async_signature::AsyncSigner` to make sure their future is `Send`able. see rust-lang/rust#115822 (comment) for more context.
|
You'll need to bump the rustc version used in CI under |
|
yeah, but when 1.75 is released right? I mostly pushed here to get that on a branch (and opened the PR for visibility), so I could pull it from another project (x509-cert). |
|
Yeah, you should be able to switch to |
|
Woohoo, it works(-ish)! |
baloo
force-pushed
the
baloo/async-signature/afit
branch
from
edfde67 to
cbc3e6c
Compare
| @@ -33,7 +34,6 @@ pub trait AsyncSigner<S: 'static> { | |||
| async fn sign_async(&self, msg: &[u8]) -> Result<S, Error>; | |||
| } | |||
|
|
|||
| #[async_trait(?Send)] | |||
| impl<S, T> AsyncSigner<S> for T | |||
There was a problem hiding this comment.
I'd much rather have a impl<D, S, T> AsyncSigner<S> for T where T: AsyncDigestSigner<D, S> instead of this blanket.
There was a problem hiding this comment.
First, you cannot write such a blanket impl, because D is unconstrained (and indeed working around that issue is the whole purpose of the signature-derive:
error[E0207]: the type parameter `D` is not constrained by the impl trait, self type, or predicates
(And though PrehashSignature can define a default digest to use with a particular S, you still won't be able to write a valid blanket impl with it, because it still won't be constrained)
Second, these traits are intended to correspond 1:1 with the traits in signature. If there isn't a blanket impl between Signer and DigestSigner, it doesn't make sense to have one here because that would be inconsistent.
Third, these blanket impls exist so that you can use a Signer as an AsyncSigner, i.e. if you have an API that accepts AsyncSigner, anyone can plug in a Signer, and the same goes for all of the other traits, eliminating the need to duplicate APIs everywhere just for async.
There was a problem hiding this comment.
I oversimplified the previous comment a bit, but yeah I intended:
impl<S, T> AsyncSigner<S> for T
where
S: PrehashSignature + 'static,
T: AsyncDigestSigner<S::Digest, S>,
{
async fn sign_async(&self, msg: &[u8]) -> Result<S, Error> {
self.sign_digest_async(S::Digest::new_with_prefix(msg))
.await
}
}This is essentially what's in the #[derive(signature::Signer)] I guess I could add that to signature_derive then (should be in the dependency tree, given the proper feature-flags)?
There was a problem hiding this comment.
That does indeed seem to work, but signature has no such blanket impl, so I think it'd be weird for them to be inconsistent.
It could potentially be included in a hypothetical future signature 3.0, although the devil is in the details for these sort of blanket impls, and they may preclude overlapping impls with a valid use case.
|
Rust 1.75 is out 🎉 |
|
Merged in #1428 |
|
Thanks! I was in a plane yesterday and couldn't follow up. |
|
@baloo let me know if v0.5.0-pre.0 works for you and I can cut a final release |
AFITis expected in the upcoming rust-1.75 release (scheduled for 2023/12/28).#[allow(async_fn_in_trait)]is required until a solution to RPITIT is merged in rust.This put responsability on the implementor of
async_signature::AsyncSignerto make sure their future isSendable.see rust-lang/rust#115822 (comment) for more context.