make scp::Msg derive(Digestible)
#127
Conversation
| proc-macro2 = "0.4.4" | ||
| quote = "0.6.3" | ||
| syn = { version = "0.15", features = [ "extra-traits" ] } | ||
| proc-macro2 = "1.0.8" |
There was a problem hiding this comment.
There are actually newer versions, but courtesy of mbedtls and its dependency on an old bindgen, we need to pin to these exact versions :(
There was a problem hiding this comment.
one thing is, there was a new version of cargo feature that can allow separating dependencies for build-time tools -- I don't know that we ever enabled it but it might help here? i'm not sure
There was a problem hiding this comment.
it sounds like it might not actually help yet anyways
There was a problem hiding this comment.
Yeah, this is caused because we can only have one version of clang-sys at a time. Right now grpc and mbedtls both need version 6, so that's the one we're stuck on.
| .iter() | ||
| .map(|variant| { | ||
| let variant_ident = &variant.ident; | ||
|
|
There was a problem hiding this comment.
I think I would want to add something like, variants.enumerate() so that we know if this is the i'th possibility, and then encode that number as a fixed width integer, e.g. a u32, and stick that in the hash before the other stuff. Something that is ensuring that there is a prefix-free representation of which variant it is before we start hitting the data associated to that value. Because the enum variant names are user generated and may not be fixed width or prefix free -- we'd like to ensure that there's no way that a hash from Option1 and Option2 can collide. that was the mechanism that I imagined would completely prevent that
|
This looks really good, thank you for doing this! |
| let expanded = quote! { | ||
| impl mc_crypto_digestible::Digestible for #ident { | ||
| impl #impl_generics mc_crypto_digestible::Digestible for #ident #ty_generics #where_clause { | ||
| fn digest<D: mc_crypto_digestible::Digest>(&self, hasher: &mut D) { | ||
| hasher.input(stringify!(#ident).as_bytes()); |
There was a problem hiding this comment.
I'm pretty sure you want to also inject the full path to each generic here as well, otherwise you can get hash collisions between, e.g. FlyingKite<Thing1> and FlyingKite<Thing2>.
I see, yeah, good point. |
This PR makes
scp::Msg(and everything else that is needed in order to support that) deriveDigestible. This would allow us to get rid of some hackish uses ofmcserial, such as this one.This required making the derive macro support generics and enums, so there are a bunch of changes around that.