polkadot-developers / substrate-docs

Substrate Developer Hub. Substrate is powered by best in class cryptographic research and comes with peer to peer networking, consensus mechanisms, and much more.
https://docs.substrate.io
BSD Zero Clause License
131 stars 275 forks source link

SCALE Option<bool> has a special encoding #2182

Closed voliva closed 1 month ago

voliva commented 1 month ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Bug report for compiling, code snippets, templates, etc.

The current Type encoding (SCALE) page explains how Options are encoded, but it's missing the special case for Option\<bool>

This was previously reported as wrong in #1061 and "a fix" was added in #1094 removing that, but the implementation of the original scale-codec still has this special case, as referenced above.

I'll be raising a PR shortly with a footer note explaining this special case, similar to the notes for Result or Compact. But what is the actual expected behaviour of SCALE?

It sounds like the previous issue that removed it from the docs was based on some nodes not having the official codec, or maybe there was a bug that changed that behaviour? Is there any place where SCALE is spec'ed?

voliva commented 1 month ago

After some investigation, it looks like indeed the nodes are encoding Option<bool> with 2 bytes.

The special implementation for Option referenced in parity-scale-codec looks like either dead code, or not being used anymore. The codec encodes Option<bool> as 2 bytes, except if you manually specify the actual OptionBool type:

use parity_scale_codec::{Encode, OptionBool};

fn main() {
    let option_1: Option<bool> = Some(false);
    let option_2: OptionBool = OptionBool(Some(false));

    // 0x0100
    println!("{}", hex::encode(&option_1.encode()));
    // 0x02
    println!("{}", hex::encode(&option_2.encode()));
}

Which means that Option actually has 2 different ways of getting encoded, according to parity-scale-codec. If this leaked out, then this would be a problem because the metadata is not able to distinguish between those two types.

I tried using OptionBool as a parameter of a RuntimeApi for a quick test to see if it's actually possible to have this alternative encoding, and fortunately, it can't be used because it lacks the TypeInfo trait.

So I think it's fair to say that OptionBool encoding is not being used currently, at least since metadata v14.