Skip to content

Fix a bug with sol_get_processed_sibling_instructions syscall #110

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wjthieme
Copy link
Contributor

The sol_get_processed_sibling_instructions for the accounts parameter, expects a pointer to an array of AccountMeta (of 34 bytes each, Pubkey + 2x bool). The AccountMeta struct in Pinocchio has a reference to the Pubkey field instead of an owned version of the Pubkey. This results in the Pinocchio AccountMeta struct only being 16 bytes. The syscall fails because of this since it would try to write data beyond the data of the supplied ptr.

This PR adds a RawAccountMeta struct which has an owned version of Pubkey, is therefore 34 bytes long and will work with the syscall.

Other option that should be possible is just define the syscall with *mut [u8] instead of creating this extra struct but imo that makes the syscall more difficult to use (you need to figure out the bytes length yourself).

@febo lmk what you think!

@wjthieme
Copy link
Contributor Author

For anyone else with this issue (I doubt it 😆). You can make the syscall work by defining a struct yourself and reinterpreting the bytes so the compiler doesn't complain

#[derive(Default, Debug, Clone)]
pub struct AccountMeta {
    pub pubkey: Pubkey,
    pub is_writable: bool,
    pub is_signer: bool,
}

// ...

let mut accounts: Vec<RawAccountMeta> = Vec::new();
let accounts_meta = &mut *(accounts.as_mut_ptr() as *mut AccountMeta);

// ...

pinocchio::syscalls::sol_get_processed_sibling_instruction(
  index,
  &mut meta,
  &mut program_id,
  data.as_mut_ptr(),
  accounts_meta,
);

@febo
Copy link
Collaborator

febo commented Mar 28, 2025

Good point, we need to fix this. We are in the process of sharing types with the SDK (see #111) so the best approach will be to use the AccountMeta from the SDK. But there is a couple of things before we get there, so your workaround is probably the best short term solution.

The current idea is that pinocchio AccountMeta becomes AccountMetaView, so there won't be a name conflict between the SDK's AccountMeta and pinocchio's one.

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

Successfully merging this pull request may close these issues.

2 participants