WrapperKeepOpaque<T> types expose wrong metadata

[eldargab]

Is there an existing issue?

• [X] I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• [X] This is not a support question.

Description of bug

For kusama chain, starting from spec version 9130, the OpaqueCall type from Multisig pallet is exposed as 2 field composite where the second field is typed as a regular Call.

This is very problematic for generic decoder, which can no longer rely on straightforward scale definitions to safely decode values containing opaque calls .

Version 9122 of kusama chain (which also uses V14 metadata) doesn't have such problem.

Steps to reproduce

No response

[bkchr]

I don't get why you can not rely on the information? The type info should be [Compact<u32>, Call], because the call is prepended with its encoded length.

[eldargab]

@bkchr , because the call wrapped with WrapperKeepOpaque might be incorrect.

For example, have a look at Multisig.as_multi happened on Shibuya. Its arguments contain completely incorrect call, yet nothing in metadata except path: ["frame_support", "traits", "misc", "WrapperKeepOpaque"] suggest it can happen.

[bkchr]

@bkchr , because the call wrapped with WrapperKeepOpaque might be incorrect.

Yes, that is the entire reason it is prepended with its length. Not sure what you now expect? How should the type express that it may fails to decode as Call?

[eldargab]

How should the type express that it may fails to decode as Call?

Any type wrapped with WrapperKeepOpaque should be exposed as Vec<u8> (as it was done before).

Imaging an indexer which ingests raw data, decodes it and stores as JSON in some database. For such indexer, WrapperKeepOpaque<T> looks like (Compact<u32>, T). It will unexpectedly fail if bytes related to T will be incorrect, which is a standard situation and supposed to be a part of normal flow.

Of course, one can say "look at type path", but such knowledge is very implicit. Rules like that are bad choice for metadata system.

[bkchr]

The initial issue why this was changed, exactly requested the opposite: https://github.com/paritytech/substrate/issues/10058

[eldargab]

The point, that WrapperKeepOpaque types can fail to decode was completely missed by https://github.com/paritytech/substrate/issues/10058.

Have a look at https://github.com/polkadot-js/api/issues/4415 to see what happened after that :)

Nevertheless, I think https://github.com/paritytech/substrate/issues/10058 relates to a broader topic of absence of any formal semantic information in substrate metadata.

For example, consider AccountId32. Generic block explorer could display such values in a more usual ss58 format, but it doesn't have good, reliable way to recognise them.

Probably, such metadata could look like

{
  tags: [
    {name: "std::AccountId32", prefix: 10},
    {name: "std::AccountId"}
  ],
  // rest type defintion fields
},
{
   tags: [
     {name: "evm::Address"}
   ],
  // ...
}

Where namespaced tag names like std::AccountId are well defined in a spec and used consistently.

[bkchr]

The point, that WrapperKeepOpaque types can fail to decode was completely missed by #10058.

This was not missed, otherwise I had switched it directly to a Call. What you are here trying to construct is something, that even not the runtime can do. The runtime also can just try to decode this data and it can fail to decode. Or it decodes it, but accidentally into some different type. Nevertheless, the only possible solution here is to try the decoding.