Vix 75 fix codama vixen account parser

[Nagaprasadvr]

Problem

• Currently vixen parser is using accounts len to match accounts and parse them but there could be issues with accounts having same length

Solution

• use account discriminators for anchor accounts and also check if acc disc exists for shank accounts and use them to match accounts and if we cannot use discs fallback to account length in the logic

[changeset-bot]

⚠️ No Changeset found

Latest commit: 38067e1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[lorisleiva]

I'm a bit confused about the approach used here.

The Codama standard was specifically designed to be framework agnostic. It offers DiscriminatorNodes that describes how accounts and instructions are recognised regardless of how that program was created.

When a Codama IDL is given, having to go back to figuring out the program framework feels like a step back and it won't benefit from any further abstraction the standard provides in the future. Nor will it benefit from updates in adapters to and from other IDLs.

If you want to generate helpers that properly leverage the Codama IDL, you have to work with DiscriminatorNodes directly. If it is an Anchor program, the DiscriminatorNodes will already have been set by the @codama/nodes-from-anchor package. There is no need to compute the hashes again.

I strongly recommend looking into how the @codama/renderers-js package deals with DiscriminatorNodes to get an idea on how to implement this properly. For instance the following files could be a good starting point: discriminatorCondition.ts and discriminatorConstants.ts.

[fernandodeluret]

Hi @lorisleiva! Hope you are doing well. Thanks for your feedback! I've kept working over this in the context of adding support for generating the grpc server related part of the generated vixen parser. In general I have tried to follow your recomendations and now the discriminator processing is abstracted from if it was a shank or anchor idl (you can look at the basic support I've added here https://github.com/abklabs/codama/blob/vix-77-add-support-to-generate-impl-for-proto-structs-and-rust/packages/renderers-vixen-parser/src/getRenderMapVisitor.ts#L305-L366).

But the one thing I feel we are missing is a default discriminator generation for shank idls, (and please correct me if I'm missing something) because the nodes-from-anchor package is setting the discriminator nodes if it's an anchor idl with the default approach for anchor discriminators, but just leaving it empty for shank idls.

So what I did so far is update the codama root object with the "common" approach for shank idls (1 byte at the start of the ix data for ixs and account length for accounts) in the generation script here https://github.com/abklabs/codama/blob/vix-77-add-support-to-generate-impl-for-proto-structs-and-rust/packages/renderers-vixen-parser/e2e/raydium-amm-v4-parser/codama.cjs#L157-L209. But that means that any user of the renderers-vixen-parser package and codama in general would have to manually set the discriminators in a similar fashion. So I would like to move that last small piece of code inside our packages for making it work out of the box for users if they don't want/need to customize anything (could be in renderers-vixen-parser because having that default it's very opinionated or also inside nodes-from-anchor). Do you have any thoughts/recommendations regarding that?

Also as a side note, I'm finishing polishing a couple of edge cases in the proto generation and once that's done the plan is create a PR from that branch I shared to include all that logic (tests are already passing and the e2e tests have the format we have been using for the new set of parsers we have released in vixen these last days)

[lorisleiva]

Hi @fernandodeluret, that's awesome! Oh well spotted for Shank IDLs that don't use the Anchor discriminator convention. In this case, I'd consider this a bug of nodes-from-anchor and I think your fix should be included there so anyone that converts a Shank or Anchor IDL can do it via the same adapter.

[fernandodeluret]

Thanks for the response @lorisleiva! I've created both PRs:

• Proto generation for Vixen Render #589 the big one with all the proto logic we have done so far
• Add support for default shank ix discriminators in nodes-from-anchor #590 the small one only adding default ix discriminators for shank idls

Also I pushed this branch using the changes from the discrminators PR for the shank example idl we had in our e2e tests and is working correctly: abklabs@1f4bac4

[Nagaprasadvr]

closing this as we have a diff pr which covers this

[lorisleiva]

Thanks guys, I'll take a look at the new PRs asap.