From 031ac3866e8460ad69507e14c8b86d3cd4ad92c3 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Thu, 6 Jun 2024 12:30:45 -0500 Subject: [PATCH] refactor get_sysvar API --- Cargo.lock | 1 - sdk/program/Cargo.toml | 1 - sdk/program/src/sysvar/mod.rs | 63 +++++++-------------------- sdk/program/src/sysvar/slot_hashes.rs | 8 +++- 4 files changed, 21 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3327d8d266042f..b1ce368a62f31c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6690,7 +6690,6 @@ dependencies = [ "solana-logger", "solana-sdk-macro", "static_assertions", - "test-case", "thiserror", "wasm-bindgen", ] diff --git a/sdk/program/Cargo.toml b/sdk/program/Cargo.toml index 30f2e8fd293ce6..ff0bac726a6b3a 100644 --- a/sdk/program/Cargo.toml +++ b/sdk/program/Cargo.toml @@ -78,7 +78,6 @@ array-bytes = { workspace = true } assert_matches = { workspace = true } serde_json = { workspace = true } static_assertions = { workspace = true } -test-case = { workspace = true } [build-dependencies] rustc_version = { workspace = true } diff --git a/sdk/program/src/sysvar/mod.rs b/sdk/program/src/sysvar/mod.rs index 5fbb27c68de004..fc3becac1b5d1a 100644 --- a/sdk/program/src/sysvar/mod.rs +++ b/sdk/program/src/sysvar/mod.rs @@ -81,12 +81,9 @@ //! //! [sysvardoc]: https://docs.solanalabs.com/runtime/sysvars +use crate::{account_info::AccountInfo, program_error::ProgramError, pubkey::Pubkey}; #[allow(deprecated)] pub use sysvar_ids::ALL_IDS; -use { - crate::{account_info::AccountInfo, program_error::ProgramError, pubkey::Pubkey}, - std::alloc::{alloc, Layout}, -}; pub mod clock; pub mod epoch_rewards; @@ -254,22 +251,20 @@ macro_rules! impl_sysvar_get { /// Handler for retrieving a slice of sysvar data from the `sol_get_sysvar` /// syscall. -fn get_sysvar<'a>(sysvar_id: &Pubkey, offset: u64, length: u64) -> Result<&'a [u8], ProgramError> { - let sysvar_id = sysvar_id as *const _ as *const u8; - - // Allocate the memory region for the sysvar data to be written to. - let var = unsafe { - let length = length as usize; - let layout = Layout::from_size_align(length, std::mem::align_of::()) - .map_err(|_| ProgramError::InvalidArgument)?; - let ptr = alloc(layout); - if ptr.is_null() { - return Err(ProgramError::InvalidArgument); - } - std::slice::from_raw_parts_mut(ptr, length) - }; +fn get_sysvar( + dst: &mut [u8], + sysvar_id: &Pubkey, + offset: u64, + length: u64, +) -> Result<(), ProgramError> { + // Check that the provided destination buffer is large enough to hold the + // requested data. + if dst.len() < length as usize { + return Err(ProgramError::InvalidArgument); + } - let var_addr = var as *mut _ as *mut u8; + let sysvar_id = sysvar_id as *const _ as *const u8; + let var_addr = dst as *mut _ as *mut u8; #[cfg(target_os = "solana")] let result = unsafe { crate::syscalls::sol_get_sysvar(sysvar_id, var_addr, offset, length) }; @@ -278,7 +273,7 @@ fn get_sysvar<'a>(sysvar_id: &Pubkey, offset: u64, length: u64) -> Result<&'a [u let result = crate::program_stubs::sol_get_sysvar(sysvar_id, var_addr, offset, length); match result { - crate::entrypoint::SUCCESS => Ok(var), + crate::entrypoint::SUCCESS => Ok(()), e => Err(e.into()), } } @@ -289,7 +284,6 @@ mod tests { super::*, crate::{clock::Epoch, program_error::ProgramError, pubkey::Pubkey}, std::{cell::RefCell, rc::Rc}, - test_case::test_case, }; #[repr(C)] @@ -342,31 +336,4 @@ mod tests { account_info.data = Rc::new(RefCell::new(&mut small_data)); assert_eq!(test_sysvar.to_account_info(&mut account_info), None); } - - #[allow(deprecated)] - #[test_case(0)] - #[test_case(clock::Clock::size_of() as u64)] - #[test_case(epoch_rewards::EpochRewards::size_of() as u64)] - #[test_case(epoch_schedule::EpochSchedule::size_of() as u64)] - #[test_case(fees::Fees::size_of() as u64)] - #[test_case(last_restart_slot::LastRestartSlot::size_of() as u64)] - #[test_case(rent::Rent::size_of() as u64)] - #[test_case(slot_hashes::SlotHashes::size_of() as u64)] - #[test_case(slot_history::SlotHistory::size_of() as u64)] - #[test_case(stake_history::StakeHistory::size_of() as u64)] - fn test_get_sysvar_alloc(length: u64) { - // As long as we use a test sysvar ID and we're _not_ testing from a BPF - // context, the `sol_get_sysvar` syscall will always return - // `ProgramError::UnsupportedSysvar`, since the ID will not match any - // sysvars in the Sysvar Cache. - // Under this condition, we can test the pointer allocation by ensuring - // the allocation routes to `ProgramError::UnsupportedSysvar` and does - // not throw a panic or `ProgramError::InvalidArgument`. - // Also, `offset` is only used in the syscall, _not_ the pointer - // allocation. - assert_eq!( - get_sysvar(&crate::sysvar::tests::id(), 0, length).unwrap_err(), - ProgramError::UnsupportedSysvar, - ); - } } diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index ada061feafbe55..d10a0b7d7cabec 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -83,7 +83,9 @@ pub trait SlotHashesSysvar { /// Get a value from the sysvar entries by its key. /// Returns `None` if the key is not found. fn get(slot: &Slot) -> Result, ProgramError> { - let data = get_sysvar(&SlotHashes::id(), 0, SlotHashes::size_of() as u64)?; + let data_len = SlotHashes::size_of(); + let mut data = vec![0u8; data_len]; + get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; let pod_hashes: &[PodSlotHash] = bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidArgument)?; @@ -96,7 +98,9 @@ pub trait SlotHashesSysvar { /// Get the position of an entry in the sysvar by its key. /// Returns `None` if the key is not found. fn position(slot: &Slot) -> Result, ProgramError> { - let data = get_sysvar(&SlotHashes::id(), 0, SlotHashes::size_of() as u64)?; + let data_len = SlotHashes::size_of(); + let mut data = vec![0u8; data_len]; + get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; let pod_hashes: &[PodSlotHash] = bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidArgument)?;