-
Notifications
You must be signed in to change notification settings - Fork 0
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
Patch the instruction discriminant to be 4 bytes #5
Conversation
...instruction, | ||
discriminant: { | ||
...instruction.discriminant, | ||
type: "u32", // The legacy native program only accepts 4-byte instruction discriminants. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paging @lorisleiva, Shank wizard. Is there a way to convince Shank to produce 4-byte instruction discriminators other than to hot-patch the idl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can maybe update the comment here to say "this program" instead of "the legacy native program", since they are both the same, but idc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly no, Shank doesn't support custom enum discriminants. You could use a Codama visitor to update this but since we're currently saving the Anchor IDL and not the Codama one, your solution is currently the best thing to do.
This can land. The program's instructions aren't serialized or deserialized using Shank, but rather bincode. loader-v3/program/src/instruction.rs Lines 378 to 382 in 1716195
This patch would just be for clients, and until Shank supports |
But hang on, if I call, say
But the new program is like:
And then we have clients sending the old data to the new program, wouldn't it get interpreted like this?
|
This is what I'm saying, the new program is not using loader-v3/program/src/processor.rs Line 25 in 1716195
In other words, it's exactly the same instruction implementation as the builtin. |
...instruction, | ||
discriminant: { | ||
...instruction.discriminant, | ||
type: "u32", // The legacy native program only accepts 4-byte instruction discriminants. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can maybe update the comment here to say "this program" instead of "the legacy native program", since they are both the same, but idc.
...instruction, | ||
discriminant: { | ||
...instruction.discriminant, | ||
type: "u32", // The legacy native program only accepts 4-byte instruction discriminants. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly no, Shank doesn't support custom enum discriminants. You could use a Codama visitor to update this but since we're currently saving the Anchor IDL and not the Codama one, your solution is currently the best thing to do.
Warning
I'm pretty sure that this is not landable, because if the legacy built-in and this program have different discriminator sizes, then the actual instruction data will not line up when we switch from one to the other. I couldn't find a way to configure Shank macros to produce 4-byte discriminants.
Fixes: #2.