Use scale-type-resolver to be generic over the type resolver used
#45
Conversation
| pub struct Field<'a> { | ||
| name: Option<&'a str>, | ||
| id: u32, | ||
| } |
There was a problem hiding this comment.
We use the Field exported from scale-type-resolver now
| pub static EMPTY_SCALE_INFO_PATH: &scale_info::Path<scale_info::form::PortableForm> = | ||
| &scale_info::Path { segments: Vec::new() }; |
There was a problem hiding this comment.
This was a bit of a hack in the first place. For now I've removed the ability to obtain the "path" to some composite type, because it's not something that's needed for decoding/encoding a type, and with the generic TypeResolver stuff it would just be an extra detail we need to try and provide from any implementation (and for our legacy type decoder I imagine it would be an unnecessary addition).
We don't provide the path to a Variant/Sequence type either anyway.
| use #path_to_scale_decode::{ Visitor, IntoVisitor }; | ||
| let val = <#path_to_type #ty_generics>::into_visitor().visit_composite(&mut composite, #path_to_scale_decode::visitor::TypeId(0)); | ||
| let val = <#path_to_type #ty_generics>::into_visitor().visit_composite(&mut composite, &Default::default()); |
There was a problem hiding this comment.
Using a type ID of 0 was a bit of a hack before, so here and in one other place we change it to being Default::default() instead. This requires our TypeId to have a Default bound on it, which ideally it would do without.
In the future I expect that we can tweak these cases and remove the bound entirely, but for now I don't suspect it will be a big deal.
| type Value<'scale, 'info> = Value; | ||
| type Error = visitor::DecodeError; | ||
| type TypeResolver = R; |
There was a problem hiding this comment.
The user facing change is basically to add this associated type, and normally to just be generic over it since you probably don't care what it is. Then, update the type signatures like below, but the logic normally stays the same.
| @@ -1043,20 +1071,21 @@ mod test { | |||
| impl Visitor for BailOutVisitor { | |||
| type Value<'scale, 'info> = (&'scale [u8], u32); | |||
| type Error = DecodeError; | |||
| type TypeResolver = PortableRegistry; | |||
There was a problem hiding this comment.
For these tests we don't care about being generic over the resolver; setting it to PortableRegistry is the "easy" change to just make everything basically the same as it was before (except that type IDs are now references in the signatures below).
| @@ -1,5 +1,5 @@ | |||
| # main codeowner @paritytech/tools-team | |||
| * @paritytech/tools-team | |||
| fn into_visitor() -> Self::Visitor { | ||
| FooVisitor | ||
| type AnyVisitor<R: TypeResolver> = FooVisitor<R>; | ||
| fn into_visitor<R: TypeResolver>() -> Self::AnyVisitor<R> { |
There was a problem hiding this comment.
DQ: This TypeResolver thingy is it implemented for all types in the scale_info::PortableTypeRegistry or how does it work?
For instance if I have some custom type, let's say for metadata before v14 is it possible that I need to implement TypeResolver for it? Is it simple to implement?
I guess it's something for subxt to handle but curious how it will look like....
There was a problem hiding this comment.
I guess it's unlikely that one needs to implement the TypeResolver trait but would good know when the T: !TypeResolver :)
There was a problem hiding this comment.
Yup, so the idea here is that any type which wants to implement IntoVisitor must do so in a way that is generic over which TypeResolver to use.
And then, for V14 metadata, scale_info::PortableRegistry is a valid choice of type resolver (it has all of the info needed to encode/decode types). For metadata before V14, users need to provide their own TypeResolver because we're not given any type info. For that resolver we'll have a map from the type names found in old metadata to the relevant info needed to encode/decode the type. The scale-info-legacy crate will define this type resolver and probably also have a load of default type mappings so that users don't need to define everything themselves (just type names unique to specific chains).
Because everything is generic over which TypeResolver to use, we can then plug in one for legacy types and use all of this same logic for decoding :)
Good point! So right now we have Probably this crate shouldn't depend on those directly, so perhaps when we revamp |
This lays the foundation for being able to use this crate in conjunction with a legacy type resolver in order to decode pre-V14 metadata. Basically, we make everything generic over how to resolve type information, and alter the decoding logic to handle this.
Once paritytech/scale-bits#4 merges and a release done, dependencies here can be updated to make things work.