paritytech/parity-scale-codec

Support enum with `Compact` prefix

Opened this issue ยท 3 comments

bkchr commented

Currently enums are using u8 as prefix. This means an enum can only have in maximum 256 variants. We need to decide if we just want to migrate everyone to Compact prefixes by default and then they can use some custom attribute to trigger the old u8 prefix. Probably the optional enabling of using Compact as prefix would be the "sane" way.

ggwpez commented

This means an enum can only have in maximum 256 variants

Is this a practical problem or more or a theoretical edge-case?

Just noting that if we change anything here, Polkadot and all parachains will need tons of migrations.

bkchr commented

This is something we want/should support. And better swallow it now than in the future :P

This currently for example means you can only have 256 pallets ๐Ÿ‘€

koute commented

We need to decide if we just want to migrate everyone to Compact prefixes by default and then they can use some custom attribute to trigger the old u8 prefix. Probably the optional enabling of using Compact as prefix would be the "sane" way.

Hmm.... perhaps something like this?

  • By default keep things working as-is.
  • At compile time check whether the variants' encoding is compatible with both formats (that is, it encodes exactly the same both as a plain old u8 and as a Compact); if it does then it compiles, if it doesn't then it spews out a compile error.
  • The user then has to explicitly tag their enum, either as being encoded using the old u8 way, or the new Compact way.

This means that for enums without tags greater or equal to 64 no source changes will be necessary.

It's not the objectively best way to do this, but would probably be the way to do it with the least amount of churn and without potentially introducing non-determinism somewhere (e.g. on a client <-> runtime boundary where both could use a different version of the codec). I don't think we should ever silently change the on-the-wire format, even in a major version bump, as that would be way too dangerous.


Also, side note: for enums specifically the way we encode compact numbers might be a little suboptimal, e.g.:

  • 0..64 -> 1 byte
  • 64..16384 -> 2 bytes
  • 16384..1073741824 -> 4 bytes

But I think it's not worth changing just to cover an extra 64 variants as a single byte.