[dynamic-codecs] Decoding empty enums and data enums should be consistent

[joefitter]

When using parseInstruction from the dynamic parsers lib, flat enum args are not mapped in the resulting data.

Eg: parsing a mpl-token-metadata revoke instruction results in data such as { discriminator: 45, revokeArgs: 9 } where revokeArgs is a simple defined enum type in the IDL.

Other instructions with more complex enum args are mapped correctly (eg unlock)

An example tx where this can be seen is 3kMGqHb8cuxZDZW6Ff7M3tsTPZannxcHuDdJzBWJ4csKNn5Ez5XKLhRb12cYR29tdpzrSg23sQ6cG8CXTmNxVKZ

[unek]

if you're parsing the idl from anchor/shank, try rootNodeFromAnchorWithoutDefaultVisitor, the default visitors flatten and break enums, tuples, and defined type links.

[joefitter]

Great, will try that thanks!

[unek]

you can drop these appropriately commented lines from the codecs package and it'll treat your empty enums as discriminated unions

this is just a quick workaround, i'll probably fix it proper soon, as i too need it to work

[unek]

pinging @lorisleiva as he's the primary contributor here and has recently been making numerous changes to enum handling in @solana/codecs-data-structures recently - perhaps getLiteralUnionCodec would be a better choice?

[lorisleiva]

try rootNodeFromAnchorWithoutDefaultVisitor, the default visitors flatten and break enums, tuples, and defined type links.

Could you please elaborate on how the default visitor affects this issue? I'd like to know if this is something we can fix and make the default visitor less intrusive.

Other instructions with more complex enum args are mapped correctly

Yes this is because when you use only empty variants, we use a different codec which is probably not the best strategy here thinking about it.

perhaps getLiteralUnionCodec would be a better choice?

Maybe the enum parser should always return something like { __kind: 'variantName', __discriminant: 42 } and if it's a struct or tuple variant, the extra fields would just be present inside this object.

[unek]

Could you please elaborate on how the default visitor affects this issue? I'd like to know if this is something we can fix and make the default visitor less intrusive.

there was an issue i had with converting jupiter v6 or phoenix idl, i believe turning off unwrapTupleEnumWithSingleStructVisitor or unwrapInstructionArgsDefinedTypesVisitor fixed the issue. just checked, this is not the case here, default visitors work fine. i'll look into it.

Maybe the enum parser should always return something like { __kind: 'variantName', __discriminant: 42 } and if it's a struct or tuple variant, the extra fields would just be present inside this object.

i think the special handling of scalar enums type nodes, which, at least with a converted idl from anchor/shank can only be a definedType (and it is what renderers expect) is unnecessary.

disabling the different codec for only empty variants you mentioned, and leaving the job to getDiscriminatedUnionCodec + getUnitCodec (which is the codec for EnumEmptyVariantType) is sufficient. decoding yields similar data to what can be seen on the explorers.

comparing with the mentioned transaction on solscan, there only is no reference to the defined type's name, but i don't think that's necessary, parsing and encoding back and forth works just fine.

[lorisleiva]

i think the special handling of scalar enums type nodes [...] is unnecessary.

I agree. But I also think we should consider including the discriminant value on top of the result provided by getDiscriminatedUnionCodec. Happy to accept a PR for it. 🙏

[unek]

i don't think it's necessary, all decoding and encoding is based on the variant name, and the byte discriminator is always the index of that variant, even the enumvarianttype's discriminator field is ignored by dynamic codecs and renderers.

also i'm not completely sure if getDiscriminatedUnionCodec is the right choice anyway for enums, according to documentation it's supposed to be rust-like enums, and i think codama's supposed to be somewhat agnostic?

[lorisleiva]

the byte discriminator is always the index of that variant

Not always, as Codama supports using custom discriminants for variants.

i don't think it's necessary

You're right though that technically, with the name of the variant and the whole Codama IDL we can infer the discriminant of that variant. It could be quite useful to expose it though since — especially for scalar enums — that can be used to compare variants from one another (e.g. Imagine a ErrorLevel enum with incremental values signifying more critical error levels).

I think codama's supposed to be somewhat agnostic?

That is true. But Codama's take on enum types is currently to support three types of variants: one that has no data, one that has an array of values and one that has an object of values. That matches perfectly with the getDiscriminatedUnionCodec of Kit.

(Note that in the future though we may just simplify the enum variant type of Codama to just be any optional nested type.)