From 133f5b93ee40cb67641f4c2040b79bc9ec8f1060 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Wed, 5 Jun 2024 19:44:44 -0500 Subject: [PATCH 01/10] sysvars: add `get_sysvar` handler --- Cargo.lock | 1 + sdk/program/Cargo.toml | 1 + sdk/program/src/sysvar/mod.rs | 64 ++++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index b1ce368a62f31c..3327d8d266042f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6690,6 +6690,7 @@ 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 ff0bac726a6b3a..30f2e8fd293ce6 100644 --- a/sdk/program/Cargo.toml +++ b/sdk/program/Cargo.toml @@ -78,6 +78,7 @@ 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 2376b371a528eb..5fbb27c68de004 100644 --- a/sdk/program/src/sysvar/mod.rs +++ b/sdk/program/src/sysvar/mod.rs @@ -81,9 +81,12 @@ //! //! [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; @@ -249,12 +252,44 @@ 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) + }; + + let var_addr = var as *mut _ as *mut u8; + + #[cfg(target_os = "solana")] + let result = unsafe { crate::syscalls::sol_get_sysvar(sysvar_id, var_addr, offset, length) }; + + #[cfg(not(target_os = "solana"))] + let result = crate::program_stubs::sol_get_sysvar(sysvar_id, var_addr, offset, length); + + match result { + crate::entrypoint::SUCCESS => Ok(var), + e => Err(e.into()), + } +} + #[cfg(test)] mod tests { use { super::*, crate::{clock::Epoch, program_error::ProgramError, pubkey::Pubkey}, std::{cell::RefCell, rc::Rc}, + test_case::test_case, }; #[repr(C)] @@ -307,4 +342,31 @@ 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, + ); + } } From 9f60e4915b9f9b400914ae57ae1648566b2f12de Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Wed, 5 Jun 2024 23:01:58 -0500 Subject: [PATCH 02/10] sysvars: add `SlotHashesSysvar` trait --- sdk/program/src/sysvar/slot_hashes.rs | 71 ++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index 18971dc48823c2..4ffdfc1bf87d78 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -46,7 +46,14 @@ //! ``` pub use crate::slot_hashes::SlotHashes; -use crate::{account_info::AccountInfo, program_error::ProgramError, sysvar::Sysvar}; +use crate::{ + account_info::AccountInfo, + clock::Slot, + hash::{Hash, HASH_BYTES}, + program_error::ProgramError, + slot_hashes::{SlotHash, MAX_ENTRIES}, + sysvar::{get_sysvar, Sysvar, SysvarId}, +}; crate::declare_sysvar_id!("SysvarS1otHashes111111111111111111111111111", SlotHashes); @@ -62,6 +69,68 @@ impl Sysvar for SlotHashes { } } +/// Trait for querying the `SlotHashes` sysvar. +pub trait SlotHashesSysvar { + /// Get a value from the sysvar entries by its key. + /// Returns `None` if the key is not found. + fn get(key: &Slot) -> Result, ProgramError> { + get_slot_hash_bytes_with_position(key) + .map(|result| result.map(|(_, bytes)| Hash::new(&bytes[8..8 + HASH_BYTES]))) + } + + /// Get the position of an entry in the sysvar by its key. + /// Returns `None` if the key is not found. + fn position(key: &Slot) -> Result, ProgramError> { + get_slot_hash_bytes_with_position(key).map(|result| result.map(|(position, _)| position)) + } +} + +impl SlotHashesSysvar for SlotHashes {} + +fn get_slot_hash_bytes_with_position<'a>( + slot: &Slot, +) -> Result, ProgramError> { + // Slot hashes is sorted largest -> smallest slot, so we can leverage + // this to perform the search. + // + // `SlotHashes` can have skipped slots, so we'll have to implement a + // binary search over data from `sol_get_sysvar`. + let key = slot.to_le_bytes(); + + // Rust's `serde::Serialize` will serialize a `usize` as a `u64` on 64-bit + // systems for vector length prefixes. + let start_offset = std::mem::size_of::(); + let length = SlotHashes::size_of().saturating_sub(start_offset); + let entry_size = std::mem::size_of::(); + + let data = get_sysvar(&SlotHashes::id(), start_offset as u64, length as u64)?; + + let mut low: usize = 0; + let mut high: usize = MAX_ENTRIES.saturating_sub(1); + while low <= high { + let mid = low.saturating_add(high).div_euclid(2); + let offset = mid.saturating_mul(entry_size); + let end = offset.saturating_add(entry_size); + + let entry_data = &data[offset..end]; + let key_data = &entry_data[..8]; + + match key_data.cmp(&key) { + std::cmp::Ordering::Equal => { + return Ok(Some((mid, entry_data))); + } + std::cmp::Ordering::Greater => { + low = mid.saturating_add(1); + } + std::cmp::Ordering::Less => { + high = mid; + } + } + } + + Ok(None) +} + #[cfg(test)] mod tests { use { From 6f5fb4630d1a2d03d36d48f2b94be8b5842c7a3d Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Thu, 6 Jun 2024 12:23:34 -0500 Subject: [PATCH 03/10] use bytemuck slices for binary search --- sdk/program/src/sysvar/slot_hashes.rs | 89 ++++++++++----------------- 1 file changed, 33 insertions(+), 56 deletions(-) diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index 4ffdfc1bf87d78..0291b492391e21 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -46,13 +46,15 @@ //! ``` pub use crate::slot_hashes::SlotHashes; -use crate::{ - account_info::AccountInfo, - clock::Slot, - hash::{Hash, HASH_BYTES}, - program_error::ProgramError, - slot_hashes::{SlotHash, MAX_ENTRIES}, - sysvar::{get_sysvar, Sysvar, SysvarId}, +use { + crate::{ + account_info::AccountInfo, + clock::Slot, + hash::Hash, + program_error::ProgramError, + sysvar::{get_sysvar, Sysvar, SysvarId}, + }, + bytemuck::{Pod, Zeroable}, }; crate::declare_sysvar_id!("SysvarS1otHashes111111111111111111111111111", SlotHashes); @@ -69,68 +71,43 @@ impl Sysvar for SlotHashes { } } +#[derive(Copy, Clone, Pod, Zeroable)] +#[repr(C)] +struct PodSlotHash { + slot: Slot, + hash: Hash, +} + /// Trait for querying the `SlotHashes` sysvar. pub trait SlotHashesSysvar { /// Get a value from the sysvar entries by its key. /// Returns `None` if the key is not found. - fn get(key: &Slot) -> Result, ProgramError> { - get_slot_hash_bytes_with_position(key) - .map(|result| result.map(|(_, bytes)| Hash::new(&bytes[8..8 + HASH_BYTES]))) + fn get(slot: &Slot) -> Result, ProgramError> { + let data = get_sysvar(&SlotHashes::id(), 0, SlotHashes::size_of() as u64)?; + let pod_hashes: &[PodSlotHash] = + bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidAccountData)?; + + Ok(pod_hashes + .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) + .map(|idx| pod_hashes[idx].hash) + .ok()) } /// Get the position of an entry in the sysvar by its key. /// Returns `None` if the key is not found. - fn position(key: &Slot) -> Result, ProgramError> { - get_slot_hash_bytes_with_position(key).map(|result| result.map(|(position, _)| position)) + fn position(slot: &Slot) -> Result, ProgramError> { + let data = get_sysvar(&SlotHashes::id(), 0, SlotHashes::size_of() as u64)?; + let pod_hashes: &[PodSlotHash] = + bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidAccountData)?; + + Ok(pod_hashes + .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) + .ok()) } } impl SlotHashesSysvar for SlotHashes {} -fn get_slot_hash_bytes_with_position<'a>( - slot: &Slot, -) -> Result, ProgramError> { - // Slot hashes is sorted largest -> smallest slot, so we can leverage - // this to perform the search. - // - // `SlotHashes` can have skipped slots, so we'll have to implement a - // binary search over data from `sol_get_sysvar`. - let key = slot.to_le_bytes(); - - // Rust's `serde::Serialize` will serialize a `usize` as a `u64` on 64-bit - // systems for vector length prefixes. - let start_offset = std::mem::size_of::(); - let length = SlotHashes::size_of().saturating_sub(start_offset); - let entry_size = std::mem::size_of::(); - - let data = get_sysvar(&SlotHashes::id(), start_offset as u64, length as u64)?; - - let mut low: usize = 0; - let mut high: usize = MAX_ENTRIES.saturating_sub(1); - while low <= high { - let mid = low.saturating_add(high).div_euclid(2); - let offset = mid.saturating_mul(entry_size); - let end = offset.saturating_add(entry_size); - - let entry_data = &data[offset..end]; - let key_data = &entry_data[..8]; - - match key_data.cmp(&key) { - std::cmp::Ordering::Equal => { - return Ok(Some((mid, entry_data))); - } - std::cmp::Ordering::Greater => { - low = mid.saturating_add(1); - } - std::cmp::Ordering::Less => { - high = mid; - } - } - } - - Ok(None) -} - #[cfg(test)] mod tests { use { From 6e9ee2c6c7e389ad5809770199474b3a8de923ab Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Thu, 6 Jun 2024 12:30:45 -0500 Subject: [PATCH 04/10] 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 0291b492391e21..aefc1cc4408394 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::InvalidAccountData)?; @@ -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::InvalidAccountData)?; From d0e431fe4d4295ae6b7c8a82f7da5a14ec6b549f Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Thu, 6 Jun 2024 17:13:44 -0500 Subject: [PATCH 05/10] add tests --- sdk/program/src/sysvar/slot_hashes.rs | 103 +++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index aefc1cc4408394..45c1be0b043573 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -116,9 +116,51 @@ impl SlotHashesSysvar for SlotHashes {} mod tests { use { super::*, - crate::{clock::Slot, hash::Hash, slot_hashes::MAX_ENTRIES}, + crate::{ + clock::Slot, + entrypoint::SUCCESS, + hash::{hash, Hash}, + program_stubs::{set_syscall_stubs, SyscallStubs}, + slot_hashes::{SlotHash, MAX_ENTRIES}, + }, }; + struct MockSlotHashesSyscall { + slot_hashes: SlotHashes, + } + + impl SyscallStubs for MockSlotHashesSyscall { + #[allow(clippy::arithmetic_side_effects)] + fn sol_get_sysvar( + &self, + _sysvar_id_addr: *const u8, + var_addr: *mut u8, + offset: u64, + length: u64, + ) -> u64 { + // The syscall tests for `sol_get_sysvar` should ensure the following: + // + // - The provided `sysvar_id_addr` can be translated into a valid + // sysvar ID for a sysvar contained in the sysvar cache, of which + // `SlotHashes` is one. + // - Length and memory checks on `offset` and `length`. + // + // Therefore this mockup can simply just unsafely use the provided + // `offset` and `length` to copy the serialized `SlotHashes` into + // the provided `var_addr`. + let data = bincode::serialize(&self.slot_hashes).unwrap(); + let slice = unsafe { std::slice::from_raw_parts_mut(var_addr, length as usize) }; + slice.copy_from_slice(&data[offset as usize..(offset + length) as usize]); + SUCCESS + } + } + + fn mock_get_sysvar_syscall(slot_hashes: &[SlotHash]) { + set_syscall_stubs(Box::new(MockSlotHashesSyscall { + slot_hashes: SlotHashes::new(slot_hashes), + })); + } + #[test] fn test_size_of() { assert_eq!( @@ -131,4 +173,63 @@ mod tests { .unwrap() as usize ); } + + #[test] + fn test_slot_hashes_sysvar() { + let mut slot_hashes = vec![]; + for i in 0..MAX_ENTRIES { + slot_hashes.push(( + i as u64, + hash(&[(i >> 24) as u8, (i >> 16) as u8, (i >> 8) as u8, i as u8]), + )); + } + + mock_get_sysvar_syscall(&slot_hashes); + + let check_slot_hashes = SlotHashes::new(&slot_hashes); + + // `get`: + assert_eq!( + ::get(&0).unwrap().as_ref(), + check_slot_hashes.get(&0), + ); + assert_eq!( + ::get(&256) + .unwrap() + .as_ref(), + check_slot_hashes.get(&256), + ); + assert_eq!( + ::get(&511) + .unwrap() + .as_ref(), + check_slot_hashes.get(&511), + ); + // `None`. + assert_eq!( + ::get(&600) + .unwrap() + .as_ref(), + check_slot_hashes.get(&600), + ); + + // `position`: + assert_eq!( + ::position(&0).unwrap(), + check_slot_hashes.position(&0), + ); + assert_eq!( + ::position(&256).unwrap(), + check_slot_hashes.position(&256), + ); + assert_eq!( + ::position(&511).unwrap(), + check_slot_hashes.position(&511), + ); + // `None`. + assert_eq!( + ::position(&600).unwrap(), + check_slot_hashes.position(&600), + ); + } } From f904e15f30b25e41d1604ff6eeab49eb61f7e974 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Thu, 6 Jun 2024 17:18:28 -0500 Subject: [PATCH 06/10] add pointer alignment check --- sdk/program/src/sysvar/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/program/src/sysvar/mod.rs b/sdk/program/src/sysvar/mod.rs index fc3becac1b5d1a..842cdd28b368c9 100644 --- a/sdk/program/src/sysvar/mod.rs +++ b/sdk/program/src/sysvar/mod.rs @@ -266,6 +266,11 @@ fn get_sysvar( let sysvar_id = sysvar_id as *const _ as *const u8; let var_addr = dst as *mut _ as *mut u8; + // Check that the provided destination buffer is aligned to 8. + if var_addr.align_offset(8) != 0 { + return Err(ProgramError::InvalidArgument); + } + #[cfg(target_os = "solana")] let result = unsafe { crate::syscalls::sol_get_sysvar(sysvar_id, var_addr, offset, length) }; From d74bd4bebea5bba463d673c9c0d7191d7471dbe1 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Fri, 7 Jun 2024 14:52:19 -0500 Subject: [PATCH 07/10] make the API a struct --- sdk/program/src/sysvar/slot_hashes.rs | 34 +++++++++++---------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index 45c1be0b043573..5009e9f55c7e42 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -78,11 +78,13 @@ struct PodSlotHash { hash: Hash, } -/// Trait for querying the `SlotHashes` sysvar. -pub trait SlotHashesSysvar { +/// API for querying the `SlotHashes` sysvar. +pub struct SlotHashesSysvar; + +impl 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> { + pub fn get(slot: &Slot) -> Result, ProgramError> { let data_len = SlotHashes::size_of(); let mut data = vec![0u8; data_len]; get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; @@ -97,7 +99,7 @@ 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> { + pub fn position(slot: &Slot) -> Result, ProgramError> { let data_len = SlotHashes::size_of(); let mut data = vec![0u8; data_len]; get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; @@ -110,8 +112,6 @@ pub trait SlotHashesSysvar { } } -impl SlotHashesSysvar for SlotHashes {} - #[cfg(test)] mod tests { use { @@ -190,45 +190,39 @@ mod tests { // `get`: assert_eq!( - ::get(&0).unwrap().as_ref(), + SlotHashesSysvar::get(&0).unwrap().as_ref(), check_slot_hashes.get(&0), ); assert_eq!( - ::get(&256) - .unwrap() - .as_ref(), + SlotHashesSysvar::get(&256).unwrap().as_ref(), check_slot_hashes.get(&256), ); assert_eq!( - ::get(&511) - .unwrap() - .as_ref(), + SlotHashesSysvar::get(&511).unwrap().as_ref(), check_slot_hashes.get(&511), ); // `None`. assert_eq!( - ::get(&600) - .unwrap() - .as_ref(), + SlotHashesSysvar::get(&600).unwrap().as_ref(), check_slot_hashes.get(&600), ); // `position`: assert_eq!( - ::position(&0).unwrap(), + SlotHashesSysvar::position(&0).unwrap(), check_slot_hashes.position(&0), ); assert_eq!( - ::position(&256).unwrap(), + SlotHashesSysvar::position(&256).unwrap(), check_slot_hashes.position(&256), ); assert_eq!( - ::position(&511).unwrap(), + SlotHashesSysvar::position(&511).unwrap(), check_slot_hashes.position(&511), ); // `None`. assert_eq!( - ::position(&600).unwrap(), + SlotHashesSysvar::position(&600).unwrap(), check_slot_hashes.position(&600), ); } From 222c5c4afb1f6ccd09c82190971358dee629ac30 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Fri, 7 Jun 2024 16:20:18 -0500 Subject: [PATCH 08/10] move allocation check --- sdk/program/src/sysvar/mod.rs | 5 ----- sdk/program/src/sysvar/slot_hashes.rs | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/sdk/program/src/sysvar/mod.rs b/sdk/program/src/sysvar/mod.rs index 842cdd28b368c9..fc3becac1b5d1a 100644 --- a/sdk/program/src/sysvar/mod.rs +++ b/sdk/program/src/sysvar/mod.rs @@ -266,11 +266,6 @@ fn get_sysvar( let sysvar_id = sysvar_id as *const _ as *const u8; let var_addr = dst as *mut _ as *mut u8; - // Check that the provided destination buffer is aligned to 8. - if var_addr.align_offset(8) != 0 { - return Err(ProgramError::InvalidArgument); - } - #[cfg(target_os = "solana")] let result = unsafe { crate::syscalls::sol_get_sysvar(sysvar_id, var_addr, offset, length) }; diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index 5009e9f55c7e42..c0dbffca6a7c21 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -87,6 +87,13 @@ impl SlotHashesSysvar { pub fn get(slot: &Slot) -> Result, ProgramError> { let data_len = SlotHashes::size_of(); let mut data = vec![0u8; data_len]; + + // Ensure the created buffer is aligned to 8. + // This should never throw unless `SlotHashes::size_of` changes. + if data.as_ptr().align_offset(8) != 0 { + return Err(ProgramError::InvalidAccountData); + } + get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; let pod_hashes: &[PodSlotHash] = bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidAccountData)?; @@ -102,6 +109,13 @@ impl SlotHashesSysvar { pub fn position(slot: &Slot) -> Result, ProgramError> { let data_len = SlotHashes::size_of(); let mut data = vec![0u8; data_len]; + + // Ensure the created buffer is aligned to 8. + // This should never throw unless `SlotHashes::size_of` changes. + if data.as_ptr().align_offset(8) != 0 { + return Err(ProgramError::InvalidAccountData); + } + get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; let pod_hashes: &[PodSlotHash] = bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidAccountData)?; From aba85df0812cc611a4f550821166a3b480444ab9 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Tue, 11 Jun 2024 13:38:19 -0500 Subject: [PATCH 09/10] use `PodSlotHash` for layout --- sdk/program/src/sysvar/slot_hashes.rs | 53 ++++++++++++++------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index c0dbffca6a7c21..1476ffead9d6b6 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -52,6 +52,7 @@ use { clock::Slot, hash::Hash, program_error::ProgramError, + slot_hashes::MAX_ENTRIES, sysvar::{get_sysvar, Sysvar, SysvarId}, }, bytemuck::{Pod, Zeroable}, @@ -71,7 +72,7 @@ impl Sysvar for SlotHashes { } } -#[derive(Copy, Clone, Pod, Zeroable)] +#[derive(Copy, Clone, Default, Pod, Zeroable)] #[repr(C)] struct PodSlotHash { slot: Slot, @@ -85,19 +86,20 @@ impl SlotHashesSysvar { /// Get a value from the sysvar entries by its key. /// Returns `None` if the key is not found. pub fn get(slot: &Slot) -> Result, ProgramError> { - let data_len = SlotHashes::size_of(); - let mut data = vec![0u8; data_len]; - - // Ensure the created buffer is aligned to 8. - // This should never throw unless `SlotHashes::size_of` changes. - if data.as_ptr().align_offset(8) != 0 { - return Err(ProgramError::InvalidAccountData); + let mut pod_hashes = vec![PodSlotHash::default(); MAX_ENTRIES]; + { + let data = bytemuck::try_cast_slice_mut::(&mut pod_hashes) + .map_err(|_| ProgramError::InvalidAccountData)?; + + // Ensure the created buffer is aligned to 8. + if data.as_ptr().align_offset(8) != 0 { + return Err(ProgramError::InvalidAccountData); + } + + let offset = 8; // Vector length as `u64`. + let length = (SlotHashes::size_of() as u64).saturating_sub(offset); + get_sysvar(data, &SlotHashes::id(), offset, length)?; } - - get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; - let pod_hashes: &[PodSlotHash] = - bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidAccountData)?; - Ok(pod_hashes .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) .map(|idx| pod_hashes[idx].hash) @@ -107,19 +109,20 @@ impl SlotHashesSysvar { /// Get the position of an entry in the sysvar by its key. /// Returns `None` if the key is not found. pub fn position(slot: &Slot) -> Result, ProgramError> { - let data_len = SlotHashes::size_of(); - let mut data = vec![0u8; data_len]; - - // Ensure the created buffer is aligned to 8. - // This should never throw unless `SlotHashes::size_of` changes. - if data.as_ptr().align_offset(8) != 0 { - return Err(ProgramError::InvalidAccountData); + let mut pod_hashes = vec![PodSlotHash::default(); MAX_ENTRIES]; + { + let data = bytemuck::try_cast_slice_mut::(&mut pod_hashes) + .map_err(|_| ProgramError::InvalidAccountData)?; + + // Ensure the created buffer is aligned to 8. + if data.as_ptr().align_offset(8) != 0 { + return Err(ProgramError::InvalidAccountData); + } + + let offset = 8; // Vector length as `u64`. + let length = (SlotHashes::size_of() as u64).saturating_sub(offset); + get_sysvar(data, &SlotHashes::id(), offset, length)?; } - - get_sysvar(&mut data, &SlotHashes::id(), 0, data_len as u64)?; - let pod_hashes: &[PodSlotHash] = - bytemuck::try_cast_slice(&data[8..]).map_err(|_| ProgramError::InvalidAccountData)?; - Ok(pod_hashes .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) .ok()) From 622edaec7348ea3a37c76a647d27e780a9b8ac8e Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Wed, 12 Jun 2024 16:20:26 -0500 Subject: [PATCH 10/10] cleanup --- sdk/program/src/sysvar/slot_hashes.rs | 71 +++++++++++++-------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/sdk/program/src/sysvar/slot_hashes.rs b/sdk/program/src/sysvar/slot_hashes.rs index 1476ffead9d6b6..9d66fab235b585 100644 --- a/sdk/program/src/sysvar/slot_hashes.rs +++ b/sdk/program/src/sysvar/slot_hashes.rs @@ -86,47 +86,41 @@ impl SlotHashesSysvar { /// Get a value from the sysvar entries by its key. /// Returns `None` if the key is not found. pub fn get(slot: &Slot) -> Result, ProgramError> { - let mut pod_hashes = vec![PodSlotHash::default(); MAX_ENTRIES]; - { - let data = bytemuck::try_cast_slice_mut::(&mut pod_hashes) - .map_err(|_| ProgramError::InvalidAccountData)?; - - // Ensure the created buffer is aligned to 8. - if data.as_ptr().align_offset(8) != 0 { - return Err(ProgramError::InvalidAccountData); - } - - let offset = 8; // Vector length as `u64`. - let length = (SlotHashes::size_of() as u64).saturating_sub(offset); - get_sysvar(data, &SlotHashes::id(), offset, length)?; - } - Ok(pod_hashes - .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) - .map(|idx| pod_hashes[idx].hash) - .ok()) + get_pod_slot_hashes().map(|pod_hashes| { + pod_hashes + .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) + .map(|idx| pod_hashes[idx].hash) + .ok() + }) } /// Get the position of an entry in the sysvar by its key. /// Returns `None` if the key is not found. pub fn position(slot: &Slot) -> Result, ProgramError> { - let mut pod_hashes = vec![PodSlotHash::default(); MAX_ENTRIES]; - { - let data = bytemuck::try_cast_slice_mut::(&mut pod_hashes) - .map_err(|_| ProgramError::InvalidAccountData)?; - - // Ensure the created buffer is aligned to 8. - if data.as_ptr().align_offset(8) != 0 { - return Err(ProgramError::InvalidAccountData); - } - - let offset = 8; // Vector length as `u64`. - let length = (SlotHashes::size_of() as u64).saturating_sub(offset); - get_sysvar(data, &SlotHashes::id(), offset, length)?; + get_pod_slot_hashes().map(|pod_hashes| { + pod_hashes + .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) + .ok() + }) + } +} + +fn get_pod_slot_hashes() -> Result, ProgramError> { + let mut pod_hashes = vec![PodSlotHash::default(); MAX_ENTRIES]; + { + let data = bytemuck::try_cast_slice_mut::(&mut pod_hashes) + .map_err(|_| ProgramError::InvalidAccountData)?; + + // Ensure the created buffer is aligned to 8. + if data.as_ptr().align_offset(8) != 0 { + return Err(ProgramError::InvalidAccountData); } - Ok(pod_hashes - .binary_search_by(|PodSlotHash { slot: this, .. }| slot.cmp(this)) - .ok()) + + let offset = 8; // Vector length as `u64`. + let length = (SlotHashes::size_of() as u64).saturating_sub(offset); + get_sysvar(data, &SlotHashes::id(), offset, length)?; } + Ok(pod_hashes) } #[cfg(test)] @@ -173,9 +167,12 @@ mod tests { } fn mock_get_sysvar_syscall(slot_hashes: &[SlotHash]) { - set_syscall_stubs(Box::new(MockSlotHashesSyscall { - slot_hashes: SlotHashes::new(slot_hashes), - })); + static ONCE: std::sync::Once = std::sync::Once::new(); + ONCE.call_once(|| { + set_syscall_stubs(Box::new(MockSlotHashesSyscall { + slot_hashes: SlotHashes::new(slot_hashes), + })); + }); } #[test]