Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support enum with Compact prefix #521

Open
bkchr opened this issue Sep 13, 2023 · 3 comments
Open

Support enum with Compact prefix #521

bkchr opened this issue Sep 13, 2023 · 3 comments

Comments

@bkchr
Copy link
Member

bkchr commented Sep 13, 2023

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.

@bkchr bkchr added this to the SCALE codec 4.0 milestone Sep 13, 2023
@ggwpez
Copy link
Member

ggwpez commented Sep 13, 2023

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
Copy link
Member Author

bkchr commented Sep 13, 2023

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
Copy link
Contributor

koute commented Sep 13, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants