Unify syscall definitions #364
Conversation
| }; | ||
| use {solana_define_syscall::define_syscall, solana_pubkey::Pubkey}; | ||
|
|
||
| define_syscall!(fn sol_get_return_data(data: *mut u8, length: u64, program_id: *mut Pubkey) -> u64); |
There was a problem hiding this comment.
To avoid a breaking change, we could consider just deprecating this definition.
| }; | ||
|
|
||
| #[cfg(target_os = "solana")] | ||
| define_syscall!(fn sol_get_processed_sibling_instruction(index: u64, meta: *mut ProcessedSiblingInstruction, program_id: *mut Pubkey, data: *mut u8, accounts: *mut AccountMeta) -> u64); |
There was a problem hiding this comment.
Same thing here, we could consider just deprecating this definition instead.
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks great to me! We're making a breaking change here anyway with #358, so let's just break the interface for these other syscalls too.
Also, since it's only function definitions, it shouldn't be too annoying for downstream users to deal with the change.
For the Another option is to write a wrapper function with the current signature over the new syscall definition. pub fn sol_get_processed_sibling_instruction(
index: u64,
meta: *mut ProcessedSiblingInstruction,
program_id: *mut Pubkey,
data: *mut u8,
accounts: *mut AccountMeta
) -> u64 {
solana_define_syscall::definitions::sol_get_processed_sibling_instruction(
index,
meta as *mut u8,
program_id as *mut u8,
data,
account_meta as *mut u8,
)
} |
Oh right, I forgot about that redefinition. Let's do a wrapping function now then, and deprecate it. I don't think anyone downstream is using it directly, but better safe than sorry. |
| buf.as_mut_ptr(), | ||
| buf.len() as u64, | ||
| &mut program_id, | ||
| &mut program_id as *mut _ as *mut u8, |
There was a problem hiding this comment.
Do you actually need two casts?
There was a problem hiding this comment.
Apparently yes, you can't go from &Pubkey to *mut u8 directly.
| // - `address` (`[u8; 32]`): account address | ||
| // - `is_signer` (`u8`): `true` if the instruction requires a signature | ||
| // - `is_writable` (`u8`): `true` if the instruction requires the account to be writable | ||
| define_syscall!(fn sol_get_processed_sibling_instruction(index: u64, meta: *mut u8, program_id: *mut u8, data: *mut u8, accounts: *mut u8) -> u64); |
There was a problem hiding this comment.
I know this is just a redefinition, but it looks unsafe to pass an array without telling the length. There is nothing to do about this, though.
There was a problem hiding this comment.
Yeah, agree. At least all syscalls are marked unsafe. 😅
|
Added deprecated wrapping functions and removed the "breaking" label. |
3 participants
Problem
While most syscall definitions are found in
define-syscall, there are two that are defined in the crate where they are used to avoid bringing dependencies. This is not ideal when other crates need access to these syscalls.Solution
Move the syscall definitions to
define-syscallcrate and use "generic" pointers.