Metadata with complete type information

[xlc]

This fixes #917, which is one of the biggest pain point to me when working with Substrate (and polkadot.js), that any type information needs to be implemented twice and incompatible type information simply cause polkadot.js crashes without much helpful clues (other than it is mostly likely a codec error).

This introduce type_registry to RuntimeMetadata which contains all the type informations i.e. the metadata itself stores the information about how to decode the storage types.

Ideally, this will remove the needs to manually maintain type information in polkadot.js (currently implemented at https://github.com/polkadot-js/api/tree/master/packages/types/src). Those JS classes can be generated at runtime by parsing the metadata. TS files can be generated ahead-of-time if needed. This will also make implement (and support) other languages SDK much easily.

Some code copied from parity-codec.

Fixes #917
Enable the proper fix for polkadot-js/api#416
Could be the foundation work for polkadot-js/api#429

This is the debug print of the resulting metadata:
https://gist.github.com/xlc/19d010e55f84bf6d43afafdc5e5b5df6#file-metadata-L5525

[parity-cla-bot]

It looks like @xlc signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

[bkchr]

Nice and ty. That was something I was thinking to do as well. Good that you started it :)
The main problem with your implementation is the blow up of human readable static stuff in the binary. In the first implementation of the metadata, we used json to represent it, but we switched to binary to get rid of all static strings lying in the runtime and increasing its size. So I would not accept your current implementation.
As I thought about it, I came to the idea of exposing fundamental types as a fixed binary representation. So, u64 could be b1 and u32 could be b01. Limiting this to 256 different values should be enough. The procedural macro could use a fixed list to generate the correct binary representation per type. And if a type is not supported, we would see this at compilation and could prepare an update of the Javascript side as well.

[xlc]

I can remove the name field and change TypeMetadata::Primative to TypeMetadata::Primative(u8), and possibility remove enum variant name and field name which should solve the concern of having human readable string in the wasm file.
However this means it can be harder to generate a developer friendly sdk / typescript file from it because there will be no more names. Maybe use some std feature flag to still have a verbose mode for native compilation?

Your idea will make breaking changes more explicit but still not solving my original issue, that update runtime module should have minimal changes to js sdk.

Let's say I want to make a runtime upgrade that introduce a new type, how to make it non-breaking change regards to existing js clients? This will happen pretty frequently when there is one team working on building a runtime module and upgrading runtime frequently. With existing Substrate and polkadot.js architecture, it is hard to quickly iterate things because someone needs to make sure polkadot.js api have the new type information, all client apps are updated to use latest api, and then an upgrade can happen. But this to me feels like defects the whole purpose or upgradeable runtime, that upgrades should be non-breaking and clients should be able to self-discovery the available features.

This may be solvable on polkadot.js side (unable to parse module information for a particular module something shouldn't break everything), or parity-codec side (somehow to make codec breaking changes harder to make, or at least detect upfront they are incompatible), but I think some changes are needed on Substrate side.

[bkchr]

First, you would just have a map:

BinaryToType = {
   0 => u64,
   1 -> u32,
    ...
}

For adding a new type, you would just need to add 2 -> i32. The javascript side could just say, values above 1 are unknown.
I think that if we support all fundamental types, there will be no missing types at all. Even if you would encode them as string, how does the javascript side knows how to interpret them?
Every higher order type will just be build up by different combinations of fundamental types. Keys would also be fundamental types with with different number of bits.
As a team will not introduce any new fundamental types to rust, the javascript side should be fairly stable, with a good initial set of known types.

[xlc]

@bkchr I have updated it to use enum (u8) to encode primitive types instead of string. Not sure what is the best way to make unrecognized primitive type become a compile-time error instead of runtime error, maybe need to rewrite impl_primatives with procedure macro?
There are more optimization could be done, such that currently if a complex type (struct/enum) is used is multiple places, it will be encoded multiple times. But those are not important for PoC purpose.

Please let me know if you think this is the correct direction to go and the best way to structure the repos. This substrate-metadata module to me can be either part of parity-codec, or a separate lib depends on parity-codec. And some method name is a bit awkward (type_metadat method was originally called metadata, which conflicts do some existing metadata method so I have to rename it), it will be good if we can come up some better and less confusing names.

I want to keep working on this but I don't want to send too much time on it and then found out it towards to a dead end.

[gavofyork]

Looks pretty cool overall (though it'll need to work with the Compact encoding stuff)

[xlc]

Needs help
I tried multiple ways but still not able to make it work with HasCompact. My Rust skill is still limited and I have trouble understand some implementation details of parity-codec and why HasCompact is implemented as it is.

Some methods I tried:
Modify parity-codec HasCompact trait, but upgrade parity-codec to master was causing some unexpected compiling error

error[E0277]: the trait bound `<<T as Trait>::Moment as codec::HasCompact>::Type: std::convert::From<<T as Trait>::Moment>` is not satisfied
   --> srml/timestamp/src/lib.rs:162:56
    |
162 |         vec![(T::TIMESTAMP_SET_POSITION, Call::set(next_time.into()))]
    |                                                              ^^^^ the trait `std::convert::From<<T as Trait>::Moment>` is not implemented for `<<T as Trait>::Moment as codec::HasCompact>::Type`
    |
    = help: consider adding a `where <<T as Trait>::Moment as codec::HasCompact>::Type: std::convert::From<<T as Trait>::Moment>` bound
    = note: required because of the requirements on the impl of `std::convert::Into<<<T as Trait>::Moment as codec::HasCompact>::Type>` for `<T as Trait>::Moment`

xlc@7ac594f

---

I tried duplicate the implementation of HasCompact in substrate-metadata module and change the runtime modules to use the duplicated and modified one, but still getting all compiling error that make me I wonder why the initial version was able to compile at all.

xlc@8b9baa0

---

I tried replace <T as HasCompac>::Type to Compac<T> and still having many unexpected compiling issues.

97c4251

---

I am also thinking move the metadata code into parity-codec, which should simplify some implementation details. Do you guys think is this something part of parity-codec?

I am stuck and need help to continue.

[bkchr]

Metadata just uses parity-codec and does not belongs to parity-codec.
I will look into your branch next week, after my holidays.

[xlc]

Latest metadata with options 1
https://gist.github.com/xlc/19d010e55f84bf6d43afafdc5e5b5df6#file-metadata-L5525

[Demi-Marie]

Nits, but I am not too familiar with this code.

[Demi-Marie]

I have no experience writing procedural macros. @rphmeier can you help review this?

[rphmeier]

I don't have much experience with proc-macros or this code in particular either.

[Demi-Marie]

is this intentional?

[xlc]

no. probably a merge error

[siman]

No automatically generated type metadata anymore?

[xlc]

We still plan to do it but unlikely within next two weeks.

[folsen]

Just a note here, the closing is not because we don't want the feature, but because the PR had gotten stale enough that it's easier to re-open fresh when ready than trying to resolve all merge conflicts, we regularly close stale issues and PRs because of uncertainty around status, so again, not an indication of desire to have it or not.

[siman]

Great!