Make all decoders `pub`

[elliot-u410]

I'm continually running into "I need this sub-decoder but it's not pub" problem. How do you feel about exposing these?

[jsdw]

So, currently none of the raw decode functions (in desub-current/src/decoder/decode_type.rs) are exposed at all.

I can definitely see a case for exposing pub fn decode_type(data: &mut &[u8], ty: &Type, types: &PortableRegistry), since that would allow you to assert (or get hold of) whatever type you'd like to decode some bytes into, and then go for it.

Would exposing that function give you what you need, or when you say "sub decoders", do you also mean the decode_composite_type, decode_variant_type etc functions?

In that case, I haven't managed to completely understand why you'd need the sub decoders yet, and I'd love to wrap my head around this need properly.

My current reasoning is that you'd always have access to a scale_info::Type (for an argument type or whatever) when attempting to decode something. Things like call data fields all have TypeId's (which you can use to get &Type's out of the registry to decode with, or you can manually construct or derive a Type (the scale_info docs are useful here) which could then be used to decode some data against.

Anyway, would love to hear more :)

It might be that we can settle on an API change to address this and #74 in one pass.

[jsdw]

(I'm exposing decode_value and decode_value_by_id now in #75, which can decode data given bytes, a Type/TypeId and the Metadata struct. Will that be sufficient to close this issue?)

[elliot-u410]

The decoder I was reaching for when I made this issue was decode_signed_extensions.

The reason I made the issue general case like this is because that practical need unveils my philosophical leaning regarding parsers like this. I don't see any reason to hide the parsers for each sub-component that is supported by desub. They are all valid on their own. They are all useful on their own. They let you build larger parsers very easily. The only reason I can think of to keep these private is if they expose some sort of API that is highly likely to change quickly or somehow has very jagged edge-cases that must be avoided. But I don't think either of these is true for the smaller parsers.

[insipx]

Yeah, I can definitely see where your coming from and making the functions like decode_signed_extensionspublic doesn't seem like it would be too much of an issue in the future imo

The only fn i see that might be problematic is decode_signature because it assumes the use of a MultiSignature type that I guess theoretically could be different depending on chain, but i'm not sure how often that actually occurs in practice

[elliot-u410]

That parser is used in several larger ones so at that point you'd have to make everything private, no?

[jsdw]

FYI I have no problem really with making the decode_x functions in the decoder module public; my only hesitation was w.r.t making the sub-Value-type decoders in the decoder::decode_type (soon to be decoder::decode_value) module public, because I see them as implementation details of the decode_type (soon to be decode_value) function and more likely to change :)

[elliot-u410]

Oh yes I can't claim I need those...

[jsdw]

The only fn i see that might be problematic is decode_signature because it assumes the use of a MultiSignature type that I guess theoretically could be different depending on chain, but i'm not sure how often that actually occurs in practice

Interesting; because the metadata doesn't know or care about that part of the signature (it only talks about the signed extension types), I was assuming that the use of MutliAddress and MultiSignature in the signature was a part of the V4 extrinsic spec and not something that could be customised. I'll have to double check that :)

[jsdw]

Re exposing decode_signature, decode_signed_extensions and decode_additional_signed. I have no strong objection to making them pub if they have some use, but if there isn't a concrete reason to expose them (they don't actually provide any additional value to anybody) I'd rather that they remained implementation details for now, to save on writing/maintaining docs for them, keep the API simpler, and give us a little more future wiggle room :)

[elliot-u410]

I guess I'll just reiterate that if you don't export them I will be living on a fork of desub indefinitely. :/

[jsdw]

Which one(s) are you making use of? And of course; if you're using them then they obviously have some use, so I'll make them pub :)

(I checked with Andrew J and MultiSignature/MultiAddress are flexible; it'll be worth eventually seeing how easy it is to be more generic over those, but I gather that most chains will stick with the defaults anyway. I'll make an issue)

[elliot-u410]

What I'm trying to do is convert an unsigned payload into a signed one. The easiest way to do this (currently) is to parse the unsigned payload myself and then re-assemble the chunks back together with the signature included. This particular use case makes use of decode_signed_extensions and decode_call_data. But I feel that these are perfectly reusable parsers that anyone could benefit from, and in particular I can foresee myself wanting later :D

[elliot-u410]

If there's still hesitation here I can happily advocate for a public but well-named module like desub::decoder::unstable. The API guarantees for such a module could be properly documented: "There are no guarantees of API stability for things in this module". I'm perfectly happy with that. As it is, desub is currently pinning exact hashes of substrate so API version compatibility doesn't seem like a high priority.

[jsdw]

Ok, that makes sense, thanks for the explanation!

I'm going to just make them all pub and export from the same module for now!

[elliot-u410]

Thank you!

[jsdw]

75 has merged now so I'll close this :)