feat!: ser/de enum discriminant

[encody]

This is a breaking change, since it will now serialize some enums (those that have manually specified discriminants) differently.

[frol]

@encody Thanks for the PR! Is it blocking you now, should I cut the release ASAP?

[encody]

@frol It is not blocking, just a matter of convenience.

[mina86]

Unless I’m misunderstanding something, this sounds like a terrible idea since users have no way to opt out of this change. Anyone who used borsh for enums which included variants with specified value is now unable to upgrade. This should have been done as a derive annotation, such as #[borsh(use_discriminants)].

[frol]

@mina86 Having a mismatch between the enum discriminant value and borsh serialized value could have silently led people to wrong assumptions. It is indeed a limitation that you cannot opt out, but why would you really? What is the use-case when you want the enum discriminant value not to match the borsh-discriminant?

[frol]

Anyone who used borsh for enums which included variants with specified value is now unable to upgrade.

The way to resolve it is to remove the explicit values assigned to the enum variants on the borsh-serialized enum and create a separate enum with the explicitly specified enum values. Overall, I agree that it is unfortunate that explicit values were not taken into account from the beginning, and now we face this breaking change.

[mina86]

What is the use-case when you want the enum discriminant value not to match the borsh-discriminant?

The use case is an existing code base with an enum where this is the case. It doesn’t matter if it makes sense. It’s also possible that authors of such code base would have preferred behaviour from this PR, but that wasn’t the behaviour so now to maintain compatibility they have to stick to the old behaviour.

The way to resolve it is to remove the explicit values assigned to the enum variants on the borsh-serialized enum and create a separate enum with the explicitly specified enum values.

Part of the problem is that this is now a silent failure. The code will compile just fine and only break in production. If upgrading to newest borsh broke compilation in places where doing so would break compatibility this would be a different matter.

[frol]

@mina86 I am happy to hear your suggestions and review PRs. Unfortunately, there are no active contributors to borsh :-(