Utilize Metadata V15

[lexnv]

This PR makes use of metadata V15, marking the stabilization of the metadata in Subxt.

• V15 is the preferred metadata version in subxt (after unstable)
• codegen utilizes the RuntimeError, RuntimeEvent and RuntimeCall directly from the metadata
• hashing and stripping of metadata are modified to reflect the outer enum types
• converting from V14 to V15 and vice-versa builds and appends the needed PortableTypes to the metadata registry
• CI link for the substrate binary is updated -- prior we were using a 9-week old release
• tests are adjusted for the latest substrate binary
  • contracts: all contracts need to import the memory from env (import "env" "memory" (memory 1 1)) contracts: switch from parity-wasm-based to wasmi-based module validation substrate#14449
  • staking: controller account soon to be deprecated; all controlling functions are moved in the stash account Staking::{bond, set_controller} to set controllers to stash only. substrate#14039 (merged about 8 weeks ago)

Testing

// Main outer enum types
"outer_enums": {
"call_enum_ty": 140,
"event_enum_ty": 20,
"error_enum_ty": 877
},

// RuntimeCall type
  "id": 140,
"type": {"path": [ "kitchensink_runtime", "RuntimeCall" ],
    "def": { "variant": { "variants":  { "name": "System",..

// System variant of the RuntimeCall. 
id": 141,
"type": { "path": [ "frame_system", "pallet", "Call",
    "def": { "variant": { "variants": [ { "name": "remark", ...

// RuntimeEvent type
"id": 20,
"type": {"path": [kitchensink_runtime", "RuntimeEvent"],
    "def": {"variant": { "variants": [ { "name": "System", ..
          
// System variant of the RuntimeEvent. 
  "id": 21,
    "type": {  "path": [ "frame_system", "pallet","Event" ],
        "def": { "variant": { "variants": [ { "name": "ExtrinsicSuccess", ..

// RuntimeError type
"id": 877,
"type": { "path": [ "kitchensink_runtime", "RuntimeError" ],
    "def": { "variant": { "variants": [ { "name": "System", ...

// System variant of the RuntimeError. 
"id": 450,
    "type": { "path": [ "frame_system","pallet", "Error"],
        "def": {"variant": { "variants": [ { "name": "InvalidSpecName", ..
             

[jsdw]

Next time it'd prolly be good to get the CI fixes in a separate PR too so we can merge them more easily, but I think this will be able to merge shortly anyway hopefully now :)

[jsdw]

Wooo, even more red than last time :D

[jsdw]

This looks super hacky, and I think it's just to populate the "type_name" thing? My question is; do we actually need this? Would anything break if we set type_name to be eg {PalletName}Error?

[lexnv]

I believe we could use the PalletError name here, have removed it :D

[lexnv]

It seems to work indeed, I have manually modified the test-runtime to convert v15 -> metadata -> v14 -> prefixed bytes; to have access in our testing to the converted version.

I was mainly looking for this test to pass, since it uses as_root_error:

subxt/testing/integration-tests/src/full_client/client/mod.rs

Line 353 in 2d0d30c

async fn decode_a_module_error() {

Changes to build.rs (with runtime-api tests disabled -- since we lose that info when going from v15 to v14):

    let metadata = RuntimeMetadataPrefixed::decode(&mut &metadata_bytes[..])
        .expect("Cannot decode frame-metadata bytes");
    let v15 = match metadata.1 {
        RuntimeMetadata::V15(v15) => v15,
        _ => panic!("Unexpected metadata version"),
    };
    let metadata: Metadata = v15.try_into().expect("Cannot convert v15 to metadata");
    let metadata: RuntimeMetadataV14 = metadata.try_into().expect("Cannot convert v15 to metadata");
    let metadata: RuntimeMetadataPrefixed = metadata.into();
    let metadata_bytes = metadata.encode();

[lexnv]

Have let the tests run with this custom patch that uses v15 converted to v14 converted back to v15 metadata, all good

[jsdw]

Ok awesome! Yeah, I had a look and type_name isn't really relevant; it's just for "disgnostic" info: https://github.com/paritytech/scale-info/blob/cc97f4c6496acf0f92d9caa45856331f094a7f38/src/ty/fields.rs#L55

The name of the field is the enum variant and the thing that decoding/encoding cares about :)

[jsdw]

Like the tests; converting to v14 and back to v15 again and checking that things line up still is a good way to check :)

[jsdw]

I'm pondering the use of visited_ids a bit now; eg is it right to pass the same visited_ids set into the call for each enum? What's the effect of doing so versus not doing so.

Doing so means that hashing things that share types might be faster; any type that's been seen will hash to some fixed "recursive" hash. The downside though is that the individual hashes become "weaker"; a bunch of more primitive types all are assigned the "recursive" hash when hashing the second and third enum. This makes them all effectively interchangeable; we'll no longer catch changes to them.

The point of visited_ids is only to prevent infinite loops, but given the above, let's pass an empty visited_ids set into each new top level call; so here get_enum_hash could create an empty Set each time for instance so that the types are entirely independently hashed.

[jsdw]

The visited_ids passed in here is only used in this one place anyway, which is good (see prev comment). Let's just pass in an &mut HashSet::new() to this function to ensure that we don't use it in some other place in the future and be consistent on "pass new visited_ids into each new top level hashing call".

[jsdw]

(looking around this file more I think we should re-think the visited_ids thing a bit or at least check that we're not overly using the same one; something I've probably done in other places too. I think ultimately we will want get_type_hash to not take in visited_ids and instead construct it internally and then an internal version of get_type_hash can accept it for the recursive calls to it. Basically, each unique type that we hash should have start from a fresh set of visited IDs. I'll open an issue to do this more thoroughly)

[jsdw]

Opened #1066 to generally fix/prevent misuse of visited_type_ids

[jsdw]

This looks awesome, nice work!

Two minor comments left for me:

• The hacky type_name thing; do we need it? (maybe? but maybe not?)
• Get rid of RawModuleError since it doesn't align with Substrate anyway now.

I wouldn't worry too much about the visited_ids stuff; we can handle that in #1066 :)

[jsdw]

Awesome stuff!