polkadot-spec icon indicating copy to clipboard operation
polkadot-spec copied to clipboard

Metadata_metadata doesn't return the SCALE-encoded metadata but the SCALE-encoded SCALE-encoded metadata

Open tomaka opened this issue 3 years ago • 6 comments

This concerns section C.5.

The title is a bit weird, but when the Metadata_metadata runtime entry point is called, Substrate encodes the metadata, producing a Vec<u8>, then encodes that Vec<u8> again, producing a different Vec<u8>.

In practice, what this means is that Metadata_metadata returns the SCALE-compact-encoded length of the SCALE-encoded metadata, followed with the SCALE-encoded metadata itself.

Note that the JSON-RPC function that returns the metadata correctly returns only the SCALE-encoded metadata. In other words it strips the length.

I would actually suggest to see if this should not be considered a Substrate bug before tweaking the spec.

tomaka avatar Oct 02 '22 08:10 tomaka

I would actually suggest to see if this should not be considered a Substrate bug before tweaking the spec.

Yes that is intended. The node side should not interpret the metadata and thus, we return SCALE encode the metadata before returning it. The runtime api itself encodes the data again to being able to decode it on the node side.

bkchr avatar Oct 02 '22 20:10 bkchr

But what you're describing is a Substrate-specific problem, which is that Substrate can't make a runtime call without decoding the output. If Substrate didn't automatically decode the output (and there's no technical reason why it would always automatically decode the output) then this would be easily solved.

tomaka avatar Oct 03 '22 07:10 tomaka

This initially substrate-specific behavior is part of the spec and mirrored in other implementations as well, so I think it is here to stay.

So if I understood this correctly, the current definition is correct under the assumption of a Runtime API call convention.

FlorianFranzen avatar Oct 03 '22 10:10 FlorianFranzen

@FlorianFranzen I guess you're "technically" correct that it's already explained in the spec. In C.3 we say that data is SCALE-encoded before being returned, and in C.5 we say that we return the SCALE-encoded metadata. So "technically" the text indeed explains that we SCALE-encode the metadata twice.

However IMO this should be explicited, as it's very subtle. As a naive reader, my assumption is that "return the SCALE-encoded metadata" means the same SCALE encoding as what section C.3 mentions, and is just repeated as a convenience.

This is IMO especially important because Metadata_metadata is a clear exception, it's the only function that SCALE encodes its return value twice.

tomaka avatar Oct 03 '22 11:10 tomaka

Also, there are several other functions in the spec, such as BabeApi_submit_report_equivocation_unsigned_extrinsic or BabeApi_generate_key_ownership_proof, where the text says "A SCALE encoded Option ...", even though the function simply returns an Option (which is then SCALE-encoded according to C.3). So to me there's a clear discrepancy here.

tomaka avatar Oct 03 '22 11:10 tomaka

But what you're describing is a Substrate-specific problem, which is that Substrate can't make a runtime call without decoding the output. If Substrate didn't automatically decode the output (and there's no technical reason why it would always automatically decode the output) then this would be easily solved.

Yeah good point, I'm okay with changing that.

bkchr avatar Oct 04 '22 08:10 bkchr