From 719e2c71510d3299e9858cd43be9c469ba4daefd Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 20 Sep 2025 19:55:14 +0100 Subject: [PATCH 01/51] Add derive address helpers --- address/Cargo.toml | 2 + address/src/derive.rs | 142 ++++++++++++++++++++++++++++++++++++++++++ address/src/lib.rs | 2 + 3 files changed, 146 insertions(+) create mode 100644 address/src/derive.rs diff --git a/address/Cargo.toml b/address/Cargo.toml index effb77807..63547cb30 100644 --- a/address/Cargo.toml +++ b/address/Cargo.toml @@ -22,6 +22,7 @@ bytemuck = ["copy", "dep:bytemuck", "dep:bytemuck_derive"] copy = [] curve25519 = ["dep:curve25519-dalek", "error", "sha2"] decode = ["dep:five8", "dep:five8_const", "error"] +derive = ["sha2-const-stable"] dev-context-only-utils = ["dep:arbitrary", "rand"] error = ["dep:solana-program-error"] frozen-abi = [ @@ -48,6 +49,7 @@ five8_const = { workspace = true, optional = true } rand = { workspace = true, optional = true } serde = { workspace = true, optional = true } serde_derive = { workspace = true, optional = true } +sha2-const-stable = { version = "0.1.0", optional = true } solana-atomic-u64 = { workspace = true, optional = true } solana-frozen-abi = { workspace = true, features = ["frozen-abi"], optional = true } solana-frozen-abi-macro = { workspace = true, features = ["frozen-abi"], optional = true } diff --git a/address/src/derive.rs b/address/src/derive.rs new file mode 100644 index 000000000..562f9f027 --- /dev/null +++ b/address/src/derive.rs @@ -0,0 +1,142 @@ +use crate::{Address, MAX_SEEDS, PDA_MARKER}; +#[cfg(target_os = "solana")] +use core::mem::MaybeUninit; + +/// Derive a [program address][pda] from the given seeds, optional bump and +/// program id. +/// +/// [pda]: https://solana.com/docs/core/pda +/// +/// In general, the derivation uses an optional bump (byte) value to ensure a +/// valid PDA (off-curve) is generated. Even when a program stores a bump to +/// derive a program address, it is necessary to use the +/// [`pinocchio::pubkey::create_program_address`] to validate the derivation. In +/// most cases, the program has the correct seeds for the derivation, so it would +/// be sufficient to just perform the derivation and compare it against the +/// expected resulting address. +/// +/// This function avoids the cost of the `create_program_address` syscall +/// (`1500` compute units) by directly computing the derived address +/// calculating the hash of the seeds, bump and program id using the +/// `sol_sha256` syscall. +/// +/// # Important +/// +/// This function differs from [`Address::create_program_address`] in that +/// it does not perform a validation to ensure that the derived address is a valid +/// (off-curve) program derived address. It is intended for use in cases where the +/// seeds, bump, and program id are known to be valid, and the caller wants to derive +/// the address without incurring the cost of the `create_program_address` syscall. +pub fn derive_address( + seeds: &[&[u8]; N], + bump: Option, + program_id: &Address, +) -> Address { + #[cfg(target_os = "solana")] + { + const { + assert!(N < MAX_SEEDS, "number of seeds must be less than MAX_SEEDS"); + } + + const UNINIT: MaybeUninit<&[u8]> = MaybeUninit::<&[u8]>::uninit(); + let mut data = [UNINIT; MAX_SEEDS + 2]; + let mut i = 0; + + while i < N { + // SAFETY: `data` is guaranteed to have enough space for `N` seeds, + // so `i` will always be within bounds. + unsafe { + data.get_unchecked_mut(i).write(seeds.get_unchecked(i)); + } + i += 1; + } + + // TODO: replace this with `as_slice` when the MSRV is upgraded + // to `1.84.0+`. + let bump_seed = [bump.unwrap_or_default()]; + + // SAFETY: `data` is guaranteed to have enough space for `MAX_SEEDS + 2` + // elements, and `MAX_SEEDS` is as large as `N`. + unsafe { + if bump.is_some() { + data.get_unchecked_mut(i).write(&bump_seed); + i += 1; + } + data.get_unchecked_mut(i).write(program_id.as_ref()); + data.get_unchecked_mut(i + 1).write(PDA_MARKER.as_ref()); + } + + let mut pda = MaybeUninit::
::uninit(); + + // SAFETY: `data` has `i + 2` elements initialized. + unsafe { + sol_sha256( + data.as_ptr() as *const u8, + (i + 2) as u64, + pda.as_mut_ptr() as *mut u8, + ); + } + + // SAFETY: `pda` has been initialized by the syscall. + Address::new_from_array(unsafe { pda.assume_init() }) + } + + #[cfg(not(target_os = "solana"))] + derive_address_const(seeds, bump, program_id) +} + +/// Derive a [program address][pda] from the given seeds, optional bump and +/// program id. +/// +/// [pda]: https://solana.com/docs/core/pda +/// +/// In general, the derivation uses an optional bump (byte) value to ensure a +/// valid PDA (off-curve) is generated. +/// +/// This function is intended for use in `const` contexts - i.e., the seeds and +/// bump are known at compile time and the program id is also a constant. It avoids +/// the cost of the `create_program_address` syscall (`1500` compute units) by +/// directly computing the derived address using the SHA-256 hash of the seeds, +/// bump and program id. +/// +/// # Important +/// +/// This function differs from [`Address::create_program_address`] in that +/// it does not perform a validation to ensure that the derived address is a valid +/// (off-curve) program derived address. It is intended for use in cases where the +/// seeds, bump, and program id are known to be valid, and the caller wants to derive +/// the address without incurring the cost of the `create_program_address` syscall. +/// +/// This function is a compile-time constant version of [`derive_address`]. +pub const fn derive_address_const( + seeds: &[&[u8]; N], + bump: Option, + program_id: &Address, +) -> Address { + const { + assert!(N < MAX_SEEDS, "number of seeds must be less than MAX_SEEDS"); + } + + let mut hasher = sha2_const_stable::Sha256::new(); + let mut i = 0; + + while i < seeds.len() { + hasher = hasher.update(seeds[i]); + i += 1; + } + + // TODO: replace this with `is_some` when the MSRV is upgraded + // to `1.84.0+`. + Address::new_from_array(if let Some(bump) = bump { + hasher + .update(&[bump]) + .update(program_id.as_array()) + .update(PDA_MARKER) + .finalize() + } else { + hasher + .update(program_id.as_array()) + .update(PDA_MARKER) + .finalize() + }) +} diff --git a/address/src/lib.rs b/address/src/lib.rs index b7bbafbc4..7120e3b5c 100644 --- a/address/src/lib.rs +++ b/address/src/lib.rs @@ -8,6 +8,8 @@ #![cfg_attr(feature = "frozen-abi", feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] +#[cfg(feature = "derive")] +pub mod derive; #[cfg(feature = "error")] pub mod error; #[cfg(feature = "rand")] From 668eff5b2bbef8c09301e670115cbed223210548 Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 20 Sep 2025 20:16:28 +0100 Subject: [PATCH 02/51] Update lock file --- Cargo.lock | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 4a43fbca0..4892921bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2632,6 +2632,12 @@ dependencies = [ "digest", ] +[[package]] +name = "sha2-const-stable" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f179d4e11094a893b82fff208f74d448a7512f99f5a0acbd5c679b705f83ed9" + [[package]] name = "sha3" version = "0.10.8" @@ -2765,6 +2771,7 @@ dependencies = [ "rand", "serde", "serde_derive", + "sha2-const-stable", "solana-account-info", "solana-address", "solana-atomic-u64", From 974ccb6bab9cfc2f2bbbd9f3d3ecb56a4af2e216 Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 20 Sep 2025 20:24:34 +0100 Subject: [PATCH 03/51] Fix doc links --- address/src/derive.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/address/src/derive.rs b/address/src/derive.rs index 562f9f027..132bb9113 100644 --- a/address/src/derive.rs +++ b/address/src/derive.rs @@ -10,7 +10,7 @@ use core::mem::MaybeUninit; /// In general, the derivation uses an optional bump (byte) value to ensure a /// valid PDA (off-curve) is generated. Even when a program stores a bump to /// derive a program address, it is necessary to use the -/// [`pinocchio::pubkey::create_program_address`] to validate the derivation. In +/// `Address::create_program_address` to validate the derivation. In /// most cases, the program has the correct seeds for the derivation, so it would /// be sufficient to just perform the derivation and compare it against the /// expected resulting address. @@ -22,7 +22,7 @@ use core::mem::MaybeUninit; /// /// # Important /// -/// This function differs from [`Address::create_program_address`] in that +/// This function differs from `Address::create_program_address` in that /// it does not perform a validation to ensure that the derived address is a valid /// (off-curve) program derived address. It is intended for use in cases where the /// seeds, bump, and program id are known to be valid, and the caller wants to derive @@ -101,7 +101,7 @@ pub fn derive_address( /// /// # Important /// -/// This function differs from [`Address::create_program_address`] in that +/// This function differs from `Address::create_program_address` in that /// it does not perform a validation to ensure that the derived address is a valid /// (off-curve) program derived address. It is intended for use in cases where the /// seeds, bump, and program id are known to be valid, and the caller wants to derive From 15b66315100199cac24e1caaa0ee99e61a219e01 Mon Sep 17 00:00:00 2001 From: febo Date: Sun, 21 Sep 2025 10:14:35 +0100 Subject: [PATCH 04/51] Add missing dependency --- address/Cargo.toml | 2 +- address/src/derive.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/address/Cargo.toml b/address/Cargo.toml index 63547cb30..9bbee4c14 100644 --- a/address/Cargo.toml +++ b/address/Cargo.toml @@ -22,7 +22,7 @@ bytemuck = ["copy", "dep:bytemuck", "dep:bytemuck_derive"] copy = [] curve25519 = ["dep:curve25519-dalek", "error", "sha2"] decode = ["dep:five8", "dep:five8_const", "error"] -derive = ["sha2-const-stable"] +derive = ["dep:sha2-const-stable", "dep:solana-define-syscall"] dev-context-only-utils = ["dep:arbitrary", "rand"] error = ["dep:solana-program-error"] frozen-abi = [ diff --git a/address/src/derive.rs b/address/src/derive.rs index 132bb9113..c8c869bdf 100644 --- a/address/src/derive.rs +++ b/address/src/derive.rs @@ -1,6 +1,6 @@ use crate::{Address, MAX_SEEDS, PDA_MARKER}; #[cfg(target_os = "solana")] -use core::mem::MaybeUninit; +use {core::mem::MaybeUninit, solana_define_syscall::definitions::sol_sha256}; /// Derive a [program address][pda] from the given seeds, optional bump and /// program id. @@ -78,7 +78,7 @@ pub fn derive_address( } // SAFETY: `pda` has been initialized by the syscall. - Address::new_from_array(unsafe { pda.assume_init() }) + unsafe { pda.assume_init() } } #[cfg(not(target_os = "solana"))] @@ -125,8 +125,8 @@ pub const fn derive_address_const( i += 1; } - // TODO: replace this with `is_some` when the MSRV is upgraded - // to `1.84.0+`. + // TODO: replace this with `bump.as_slice()` when the MSRV is + // upgraded to `1.84.0+`. Address::new_from_array(if let Some(bump) = bump { hasher .update(&[bump]) From 036ecc8e0184129cec7368b619e4116ea162182a Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Tue, 24 Sep 2024 16:30:10 +0100 Subject: [PATCH 05/51] Refactor directory structure (#18) * Use macro rules * Update directory structure --- account-view/src/lib.rs | 419 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 419 insertions(+) create mode 100644 account-view/src/lib.rs diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs new file mode 100644 index 000000000..1a430f0c5 --- /dev/null +++ b/account-view/src/lib.rs @@ -0,0 +1,419 @@ +//! Data structures to represent account information. + +use core::{ptr::NonNull, slice::from_raw_parts_mut}; + +use crate::{program_error::ProgramError, pubkey::Pubkey, syscalls::sol_memset_}; + +/// Maximum number of bytes a program may add to an account during a +/// single realloc. +pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; + +/// Raw account data. +/// +/// This data is wrapped in an `AccountInfo` struct, which provides safe access +/// to the data. +#[repr(C)] +#[derive(Clone, Copy, Default)] +pub(crate) struct Account { + /// Borrow state of the account data. + /// + /// 0) We reuse the duplicate flag for this. We set it to 0b0000_0000. + /// 1) We use the first four bits to track state of lamport borrow + /// 2) We use the second four bits to track state of data borrow + /// + /// 4 bit state: [1 bit mutable borrow flag | u3 immmutable borrow flag] + /// This gives us up to 7 immutable borrows. Note that does not mean 7 + /// duplicate account infos, but rather 7 calls to borrow lamports or + /// borrow data across all duplicate account infos. + pub(crate) borrow_state: u8, + + /// Indicates whether the transaction was signed by this account. + is_signer: u8, + + /// Indicates whether the account is writable. + is_writable: u8, + + /// Indicates whether this account represents a program. + executable: u8, + + /// Account's original data length when it was serialized for the + /// current program invocation. + /// + /// The value of this field is lazily initialized to the current data length. + /// On first access, the original data length will be 0. The caller should + /// ensure that the original data length is set to the current data length for + /// subsequence access. + /// + /// The value of this field is currently only used for `realloc`. + original_data_len: [u8; 4], + + /// Public key of the account + key: Pubkey, + + /// Program that owns this account + owner: Pubkey, + + /// The lamports in the account. Modifiable by programs. + lamports: u64, + + /// Length of the data. + pub(crate) data_len: u64, +} + +// Convenience macro to get the original data length from the account – the value will +// be zero on first access. +macro_rules! get_original_data_len { + ( $self:expr ) => { + unsafe { *(&(*$self).original_data_len as *const _ as *const u32) as usize } + }; +} + +// Convenience macro to set the original data length in the account. +macro_rules! set_original_data_len { + ( $self:expr, $len:expr ) => { + unsafe { + *(&mut (*$self).original_data_len) = u32::to_le_bytes($len as u32); + } + }; +} + +/// Wrapper struct for an `Account`. +/// +/// This struct provides safe access to the data in an `Account`. It is also +/// used to track borrows of the account data and lamports, given that an +/// account can be "shared" across multiple `AccountInfo` instances. +#[repr(C)] +#[derive(Clone, PartialEq, Eq)] +pub struct AccountInfo { + /// Raw (pointer to) account data. + /// + /// Note that this is a pointer can be shared across multiple `AccountInfo`. + pub(crate) raw: *mut Account, +} + +impl AccountInfo { + /// Public key of the account. + #[inline(always)] + pub fn key(&self) -> &Pubkey { + unsafe { &(*self.raw).key } + } + + /// Program that owns this account. + #[inline(always)] + pub fn owner(&self) -> &Pubkey { + unsafe { &(*self.raw).owner } + } + + /// Indicates whether the transaction was signed by this account. + #[inline(always)] + pub fn is_signer(&self) -> bool { + unsafe { (*self.raw).is_signer != 0 } + } + + /// Indicates whether the account is writable. + #[inline(always)] + pub fn is_writable(&self) -> bool { + unsafe { (*self.raw).is_writable != 0 } + } + + /// Indicates whether this account represents a program. + /// + /// Program accounts are always read-only. + #[inline(always)] + pub fn executable(&self) -> bool { + unsafe { (*self.raw).executable != 0 } + } + + /// Returns the size of the data in the account. + #[inline(always)] + pub fn data_len(&self) -> usize { + unsafe { (*self.raw).data_len as usize } + } + + /// Indicates whether the account data is empty. + /// + /// An account is considered empty if the data length is zero. + #[inline(always)] + pub fn data_is_empty(&self) -> bool { + self.data_len() == 0 + } + + /// Changes the owner of the account. + #[allow(invalid_reference_casting)] + pub fn assign(&self, new_owner: &Pubkey) { + unsafe { + core::ptr::write_volatile(&(*self.raw).owner as *const _ as *mut Pubkey, *new_owner); + } + } + + /// Returns a read-only reference to the lamports in the account. + /// + /// # Safety + /// + /// This does not check or modify the 4-bit refcell. Useful when instruction + /// has verified non-duplicate accounts. + pub unsafe fn unchecked_borrow_lamports(&self) -> &u64 { + &(*self.raw).lamports + } + + /// Returns a mutable reference to the lamports in the account. + /// + /// # Safety + /// + /// This does not check or modify the 4-bit refcell. Useful when instruction + /// has verified non-duplicate accounts. + #[allow(clippy::mut_from_ref)] + pub unsafe fn unchecked_borrow_mut_lamports(&self) -> &mut u64 { + &mut (*self.raw).lamports + } + + /// Returns a read-only reference to the data in the account. + /// + /// # Safety + /// + /// This does not check or modify the 4-bit refcell. Useful when instruction + /// has verified non-duplicate accounts. + pub unsafe fn unchecked_borrow_data(&self) -> &[u8] { + core::slice::from_raw_parts(self.data_ptr(), self.data_len()) + } + + /// Returns a mutable reference to the data in the account. + /// + /// # Safety + /// + /// This does not check or modify the 4-bit refcell. Useful when instruction + /// has verified non-duplicate accounts. + #[allow(clippy::mut_from_ref)] + pub unsafe fn unchecked_borrow_mut_data(&self) -> &mut [u8] { + core::slice::from_raw_parts_mut(self.data_ptr(), self.data_len()) + } + + /// Tries to get a read-only reference to the lamport field, failing if the + /// field is already mutable borrowed or if 7 borrows already exist. + pub fn try_borrow_lamports(&self) -> Result, ProgramError> { + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; + + // check if mutable borrow is already taken + if *borrow_state & 0b_1000_0000 != 0 { + return Err(ProgramError::AccountBorrowFailed); + } + + // check if we have reached the max immutable borrow count + if *borrow_state & 0b_0111_0000 == 0b_0111_0000 { + return Err(ProgramError::AccountBorrowFailed); + } + + // increment the immutable borrow count + *borrow_state += 1 << LAMPORTS_SHIFT; + + // return the reference to lamports + Ok(Ref { + value: unsafe { &(*self.raw).lamports }, + state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + borrow_shift: LAMPORTS_SHIFT, + }) + } + + /// Tries to get a read only reference to the lamport field, failing if the field + /// is already borrowed in any form. + pub fn try_borrow_mut_lamports(&self) -> Result, ProgramError> { + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; + + // check if any borrow (mutable or immutable) is already taken for lamports + if *borrow_state & 0b_1111_0000 != 0 { + return Err(ProgramError::AccountBorrowFailed); + } + + // set the mutable lamport borrow flag + *borrow_state |= 0b_1000_0000; + + // return the mutable reference to lamports + Ok(RefMut { + value: unsafe { &mut (*self.raw).lamports }, + state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + borrow_mask: LAMPORTS_MASK, + }) + } + + /// Tries to get a read only reference to the data field, failing if the field + /// is already mutable borrowed or if 7 borrows already exist. + pub fn try_borrow_data(&self) -> Result, ProgramError> { + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; + + // check if mutable data borrow is already taken (most significant bit + // of the data_borrow_state) + if *borrow_state & 0b_0000_1000 != 0 { + return Err(ProgramError::AccountBorrowFailed); + } + + // check if we have reached the max immutable data borrow count (7) + if *borrow_state & 0b0111 == 0b0111 { + return Err(ProgramError::AccountBorrowFailed); + } + + // increment the immutable data borrow count + *borrow_state += 1; + + // return the reference to data + Ok(Ref { + value: unsafe { core::slice::from_raw_parts(self.data_ptr(), self.data_len()) }, + state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + borrow_shift: DATA_SHIFT, + }) + } + + /// Tries to get a read only reference to the data field, failing if the field + /// is already borrowed in any form. + pub fn try_borrow_mut_data(&self) -> Result, ProgramError> { + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; + + // check if any borrow (mutable or immutable) is already taken for data + if *borrow_state & 0b_0000_1111 != 0 { + return Err(ProgramError::AccountBorrowFailed); + } + + // set the mutable data borrow flag + *borrow_state |= 0b0000_1000; + + // return the mutable reference to data + Ok(RefMut { + value: unsafe { from_raw_parts_mut(self.data_ptr(), self.data_len()) }, + state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + borrow_mask: DATA_MASK, + }) + } + + /// Realloc the account's data and optionally zero-initialize the new + /// memory. + /// + /// Note: Account data can be increased within a single call by up to + /// `solana_program::entrypoint::MAX_PERMITTED_DATA_INCREASE` bytes. + /// + /// Note: Memory used to grow is already zero-initialized upon program + /// entrypoint and re-zeroing it wastes compute units. If within the same + /// call a program reallocs from larger to smaller and back to larger again + /// the new space could contain stale data. Pass `true` for `zero_init` in + /// this case, otherwise compute units will be wasted re-zero-initializing. + /// + /// # Safety + /// + /// This method makes assumptions about the layout and location of memory + /// referenced by `AccountInfo` fields. It should only be called for + /// instances of `AccountInfo` that were created by the runtime and received + /// in the `process_instruction` entrypoint of a program. + pub fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), ProgramError> { + let mut data = self.try_borrow_mut_data()?; + let current_len = data.len(); + + // return early if length hasn't changed + if new_len == current_len { + return Ok(()); + } + + let original_len = match get_original_data_len!(self.raw) { + len if len > 0 => len, + _ => { + set_original_data_len!(self.raw, current_len); + current_len + } + }; + + // return early if the length increase from the original serialized data + // length is too large and would result in an out of bounds allocation + if new_len.saturating_sub(original_len) > MAX_PERMITTED_DATA_INCREASE { + return Err(ProgramError::InvalidRealloc); + } + + // realloc + unsafe { + let data_ptr = data.as_mut_ptr(); + // set new length in the serialized data + *(data_ptr.offset(-8) as *mut u64) = new_len as u64; + // recreate the local slice with the new length + data.value = from_raw_parts_mut(data_ptr, new_len); + } + + if zero_init { + let len_increase = new_len.saturating_sub(current_len); + if len_increase > 0 { + unsafe { + sol_memset_( + &mut data[original_len..] as *mut _ as *mut u8, + 0, + len_increase as u64, + ); + } + } + } + + Ok(()) + } + + /// Returns the memory address of the account data. + fn data_ptr(&self) -> *mut u8 { + unsafe { (self.raw as *const _ as *mut u8).add(core::mem::size_of::()) } + } +} + +/// Bytes to shift to get to the borrow state of lamports. +const LAMPORTS_SHIFT: u8 = 4; + +/// Bytes to shift to get to the borrow state of data. +const DATA_SHIFT: u8 = 0; + +/// Reference to account data or lamports with checked borrow rules. +pub struct Ref<'a, T: ?Sized> { + value: &'a T, + state: NonNull, + /// Indicates the type of borrow (lamports or data) by representing the + /// shift amount. + borrow_shift: u8, +} + +impl<'a, T: ?Sized> core::ops::Deref for Ref<'a, T> { + type Target = T; + fn deref(&self) -> &Self::Target { + self.value + } +} + +impl<'a, T: ?Sized> Drop for Ref<'a, T> { + // decrement the immutable borrow count + fn drop(&mut self) { + unsafe { *self.state.as_mut() -= 1 << self.borrow_shift }; + } +} + +/// Mask representing the mutable borrow flag for lamports. +const LAMPORTS_MASK: u8 = 0b_0111_1111; + +/// Mask representing the mutable borrow flag for data. +const DATA_MASK: u8 = 0b_1111_0111; + +/// Mutable reference to account data or lamports with checked borrow rules. +pub struct RefMut<'a, T: ?Sized> { + value: &'a mut T, + state: NonNull, + /// Indicates the type of borrow (lamports or data) by representing the + /// mutable borrow mask. + borrow_mask: u8, +} + +impl<'a, T: ?Sized> core::ops::Deref for RefMut<'a, T> { + type Target = T; + fn deref(&self) -> &Self::Target { + self.value + } +} +impl<'a, T: ?Sized> core::ops::DerefMut for RefMut<'a, T> { + fn deref_mut(&mut self) -> &mut ::Target { + self.value + } +} + +impl<'a, T: ?Sized> Drop for RefMut<'a, T> { + // unset the mutable borrow flag + fn drop(&mut self) { + unsafe { *self.state.as_mut() &= self.borrow_mask }; + } +} From 35146d4f84a70a5916808e5b0574fcfdb6111794 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Tue, 1 Oct 2024 23:09:34 +0100 Subject: [PATCH 06/51] Add account info and pubkey helpers (#21) --- account-view/src/lib.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 1a430f0c5..d237b34da 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -8,6 +8,16 @@ use crate::{program_error::ProgramError, pubkey::Pubkey, syscalls::sol_memset_}; /// single realloc. pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; +#[macro_export] +macro_rules! get_account_info { + ( $accounts:ident, $index:expr ) => {{ + if $accounts.len() <= $index { + return Err($crate::program_error::ProgramError::NotEnoughAccountKeys); + } + &$accounts[$index] + }}; +} + /// Raw account data. /// /// This data is wrapped in an `AccountInfo` struct, which provides safe access From de8cb47723ae9f52d47a6606ddf25bf0e452c129 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Sat, 5 Oct 2024 01:10:47 +0100 Subject: [PATCH 07/51] Rename unchecked methods (#22) --- account-view/src/lib.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index d237b34da..fd8326d67 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -160,9 +160,9 @@ impl AccountInfo { /// /// # Safety /// - /// This does not check or modify the 4-bit refcell. Useful when instruction - /// has verified non-duplicate accounts. - pub unsafe fn unchecked_borrow_lamports(&self) -> &u64 { + /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow + /// flag untouched. Useful when an instruction has verified non-duplicate accounts. + pub unsafe fn borrow_lamports_unchecked(&self) -> &u64 { &(*self.raw).lamports } @@ -170,10 +170,10 @@ impl AccountInfo { /// /// # Safety /// - /// This does not check or modify the 4-bit refcell. Useful when instruction - /// has verified non-duplicate accounts. + /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow + /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[allow(clippy::mut_from_ref)] - pub unsafe fn unchecked_borrow_mut_lamports(&self) -> &mut u64 { + pub unsafe fn borrow_mut_lamports_unchecked(&self) -> &mut u64 { &mut (*self.raw).lamports } @@ -181,9 +181,9 @@ impl AccountInfo { /// /// # Safety /// - /// This does not check or modify the 4-bit refcell. Useful when instruction - /// has verified non-duplicate accounts. - pub unsafe fn unchecked_borrow_data(&self) -> &[u8] { + /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow + /// flag untouched. Useful when an instruction has verified non-duplicate accounts. + pub unsafe fn borrow_data_unchecked(&self) -> &[u8] { core::slice::from_raw_parts(self.data_ptr(), self.data_len()) } @@ -191,10 +191,10 @@ impl AccountInfo { /// /// # Safety /// - /// This does not check or modify the 4-bit refcell. Useful when instruction - /// has verified non-duplicate accounts. + /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow + /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[allow(clippy::mut_from_ref)] - pub unsafe fn unchecked_borrow_mut_data(&self) -> &mut [u8] { + pub unsafe fn borrow_mut_data_unchecked(&self) -> &mut [u8] { core::slice::from_raw_parts_mut(self.data_ptr(), self.data_len()) } @@ -219,7 +219,7 @@ impl AccountInfo { // return the reference to lamports Ok(Ref { value: unsafe { &(*self.raw).lamports }, - state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_shift: LAMPORTS_SHIFT, }) } @@ -240,7 +240,7 @@ impl AccountInfo { // return the mutable reference to lamports Ok(RefMut { value: unsafe { &mut (*self.raw).lamports }, - state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_mask: LAMPORTS_MASK, }) } @@ -257,7 +257,7 @@ impl AccountInfo { } // check if we have reached the max immutable data borrow count (7) - if *borrow_state & 0b0111 == 0b0111 { + if *borrow_state & 0b_0111 == 0b0111 { return Err(ProgramError::AccountBorrowFailed); } @@ -267,7 +267,7 @@ impl AccountInfo { // return the reference to data Ok(Ref { value: unsafe { core::slice::from_raw_parts(self.data_ptr(), self.data_len()) }, - state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_shift: DATA_SHIFT, }) } @@ -283,12 +283,12 @@ impl AccountInfo { } // set the mutable data borrow flag - *borrow_state |= 0b0000_1000; + *borrow_state |= 0b_0000_1000; // return the mutable reference to data Ok(RefMut { value: unsafe { from_raw_parts_mut(self.data_ptr(), self.data_len()) }, - state: unsafe { NonNull::new_unchecked(&mut (*self.raw).borrow_state) }, + state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_mask: DATA_MASK, }) } From dacd11cb45e2c966508f2f470b050c97833612d3 Mon Sep 17 00:00:00 2001 From: "Jean Marchand (Exotic Markets)" Date: Wed, 16 Oct 2024 20:36:50 +0700 Subject: [PATCH 08/51] Add map and filter_map to Ref and RefMut (#27) * Add map and filter_map to Ref and RefMut * Add unit tests * Apply suggestions --- account-view/src/lib.rs | 240 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 229 insertions(+), 11 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index fd8326d67..56b0d9aef 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,6 +1,6 @@ //! Data structures to represent account information. -use core::{ptr::NonNull, slice::from_raw_parts_mut}; +use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull, slice::from_raw_parts_mut}; use crate::{program_error::ProgramError, pubkey::Pubkey, syscalls::sol_memset_}; @@ -218,9 +218,10 @@ impl AccountInfo { // return the reference to lamports Ok(Ref { - value: unsafe { &(*self.raw).lamports }, + value: unsafe { NonNull::from(&(*self.raw).lamports) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_shift: LAMPORTS_SHIFT, + marker: PhantomData, }) } @@ -239,9 +240,10 @@ impl AccountInfo { // return the mutable reference to lamports Ok(RefMut { - value: unsafe { &mut (*self.raw).lamports }, + value: unsafe { NonNull::from(&mut (*self.raw).lamports) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_mask: LAMPORTS_MASK, + marker: PhantomData, }) } @@ -266,9 +268,15 @@ impl AccountInfo { // return the reference to data Ok(Ref { - value: unsafe { core::slice::from_raw_parts(self.data_ptr(), self.data_len()) }, + value: unsafe { + NonNull::from(core::slice::from_raw_parts( + self.data_ptr(), + self.data_len(), + )) + }, state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_shift: DATA_SHIFT, + marker: PhantomData, }) } @@ -287,9 +295,10 @@ impl AccountInfo { // return the mutable reference to data Ok(RefMut { - value: unsafe { from_raw_parts_mut(self.data_ptr(), self.data_len()) }, + value: unsafe { NonNull::from(from_raw_parts_mut(self.data_ptr(), self.data_len())) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_mask: DATA_MASK, + marker: PhantomData, }) } @@ -340,7 +349,7 @@ impl AccountInfo { // set new length in the serialized data *(data_ptr.offset(-8) as *mut u64) = new_len as u64; // recreate the local slice with the new length - data.value = from_raw_parts_mut(data_ptr, new_len); + data.value = NonNull::from(from_raw_parts_mut(data_ptr, new_len)); } if zero_init { @@ -373,17 +382,57 @@ const DATA_SHIFT: u8 = 0; /// Reference to account data or lamports with checked borrow rules. pub struct Ref<'a, T: ?Sized> { - value: &'a T, + value: NonNull, state: NonNull, /// Indicates the type of borrow (lamports or data) by representing the /// shift amount. borrow_shift: u8, + /// The `value` raw pointer is only valid while the `&'a T` lives so we claim + /// to hold a reference to it. + marker: PhantomData<&'a T>, +} + +impl<'a, T: ?Sized> Ref<'a, T> { + #[inline] + pub fn map(orig: Ref<'a, T>, f: F) -> Ref<'a, U> + where + F: FnOnce(&T) -> &U, + { + // Avoid decrementing the borrow flag on Drop. + let orig = ManuallyDrop::new(orig); + + Ref { + value: NonNull::from(f(&*orig)), + state: orig.state, + borrow_shift: orig.borrow_shift, + marker: PhantomData, + } + } + + #[inline] + pub fn filter_map(orig: Ref<'a, T>, f: F) -> Result, Self> + where + F: FnOnce(&T) -> Option<&U>, + { + // Avoid decrementing the borrow flag on Drop. + let orig = ManuallyDrop::new(orig); + + match f(&*orig) { + Some(value) => Ok(Ref { + value: NonNull::from(value), + state: orig.state, + borrow_shift: orig.borrow_shift, + marker: PhantomData, + }), + None => Err(ManuallyDrop::into_inner(orig)), + } + } } impl<'a, T: ?Sized> core::ops::Deref for Ref<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { - self.value + unsafe { self.value.as_ref() } } } @@ -402,22 +451,65 @@ const DATA_MASK: u8 = 0b_1111_0111; /// Mutable reference to account data or lamports with checked borrow rules. pub struct RefMut<'a, T: ?Sized> { - value: &'a mut T, + value: NonNull, state: NonNull, /// Indicates the type of borrow (lamports or data) by representing the /// mutable borrow mask. borrow_mask: u8, + /// The `value` raw pointer is only valid while the `&'a T` lives so we claim + /// to hold a reference to it. + marker: PhantomData<&'a mut T>, +} + +impl<'a, T: ?Sized> RefMut<'a, T> { + #[inline] + pub fn map(orig: RefMut<'a, T>, f: F) -> RefMut<'a, U> + where + F: FnOnce(&mut T) -> &mut U, + { + // Avoid decrementing the borrow flag on Drop. + let mut orig = ManuallyDrop::new(orig); + + RefMut { + value: NonNull::from(f(&mut *orig)), + state: orig.state, + borrow_mask: orig.borrow_mask, + marker: PhantomData, + } + } + + #[inline] + pub fn filter_map(orig: RefMut<'a, T>, f: F) -> Result, Self> + where + F: FnOnce(&mut T) -> Option<&mut U>, + { + // Avoid decrementing the mutable borrow flag on Drop. + let mut orig = ManuallyDrop::new(orig); + + match f(&mut *orig) { + Some(value) => { + let value = NonNull::from(value); + Ok(RefMut { + value, + state: orig.state, + borrow_mask: orig.borrow_mask, + marker: PhantomData, + }) + } + None => Err(ManuallyDrop::into_inner(orig)), + } + } } impl<'a, T: ?Sized> core::ops::Deref for RefMut<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { - self.value + unsafe { self.value.as_ref() } } } impl<'a, T: ?Sized> core::ops::DerefMut for RefMut<'a, T> { fn deref_mut(&mut self) -> &mut ::Target { - self.value + unsafe { self.value.as_mut() } } } @@ -427,3 +519,129 @@ impl<'a, T: ?Sized> Drop for RefMut<'a, T> { unsafe { *self.state.as_mut() &= self.borrow_mask }; } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_data_ref() { + let data: [u8; 4] = [0, 1, 2, 3]; + let state = 1 << DATA_SHIFT; + + let ref_data = Ref { + value: NonNull::from(&data), + borrow_shift: DATA_SHIFT, + state: NonNull::from(&state), + marker: PhantomData, + }; + + let new_ref = Ref::map(ref_data, |data| &data[1]); + + assert_eq!(state, 1 << DATA_SHIFT); + assert_eq!(*new_ref, 1); + + let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&3)) else { + unreachable!() + }; + + assert_eq!(state, 1 << DATA_SHIFT); + assert_eq!(*new_ref, 3); + + let new_ref = Ref::filter_map(new_ref, |_| Option::<&u8>::None); + + assert_eq!(state, 1 << DATA_SHIFT); + assert!(new_ref.is_err()); + + drop(new_ref); + + assert_eq!(state, 0 << DATA_SHIFT); + } + + #[test] + fn test_lamports_ref() { + let lamports: u64 = 10000; + let state = 1 << LAMPORTS_SHIFT; + + let ref_lamports = Ref { + value: NonNull::from(&lamports), + borrow_shift: LAMPORTS_SHIFT, + state: NonNull::from(&state), + marker: PhantomData, + }; + + let new_ref = Ref::map(ref_lamports, |_| &1000); + + assert_eq!(state, 1 << LAMPORTS_SHIFT); + assert_eq!(*new_ref, 1000); + + let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&2000)) else { + unreachable!() + }; + + assert_eq!(state, 1 << LAMPORTS_SHIFT); + assert_eq!(*new_ref, 2000); + + let new_ref = Ref::filter_map(new_ref, |_| Option::<&i32>::None); + + assert_eq!(state, 1 << LAMPORTS_SHIFT); + assert!(new_ref.is_err()); + + drop(new_ref); + + assert_eq!(state, 0 << LAMPORTS_SHIFT); + } + + #[test] + fn test_data_ref_mut() { + let data: [u8; 4] = [0, 1, 2, 3]; + let state = 0b_0000_1000; + + let ref_data = RefMut { + value: NonNull::from(&data), + borrow_mask: DATA_MASK, + state: NonNull::from(&state), + marker: PhantomData, + }; + + let Ok(mut new_ref) = RefMut::filter_map(ref_data, |data| data.get_mut(0)) else { + unreachable!() + }; + + *new_ref = 4; + + assert_eq!(state, 8); + assert_eq!(*new_ref, 4); + + drop(new_ref); + + assert_eq!(data, [4, 1, 2, 3]); + assert_eq!(state, 0); + } + + #[test] + fn test_lamports_ref_mut() { + let lamports: u64 = 10000; + let state = 0b_1000_0000; + + let ref_lamports = RefMut { + value: NonNull::from(&lamports), + borrow_mask: LAMPORTS_MASK, + state: NonNull::from(&state), + marker: PhantomData, + }; + + let new_ref = RefMut::map(ref_lamports, |lamports| { + *lamports = 200; + lamports + }); + + assert_eq!(state, 128); + assert_eq!(*new_ref, 200); + + drop(new_ref); + + assert_eq!(lamports, 200); + assert_eq!(state, 0); + } +} From 663332db8c1237c8a4fee961581c0de126910db6 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Sun, 20 Oct 2024 01:09:28 +0100 Subject: [PATCH 09/51] Add bit flag for original data length (#28) * Add bit flag * Remove declarative macro --- account-view/src/lib.rs | 57 ++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 56b0d9aef..9a5d2edf0 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -49,13 +49,15 @@ pub(crate) struct Account { /// Account's original data length when it was serialized for the /// current program invocation. /// - /// The value of this field is lazily initialized to the current data length. - /// On first access, the original data length will be 0. The caller should - /// ensure that the original data length is set to the current data length for - /// subsequence access. + /// The value of this field is lazily initialized to the current data length + /// and the [`SET_LEN_MASK`] flag on first access. When reading this field, + /// the flag is cleared to retrieve the original data length by using the + /// [`GET_LEN_MASK`] mask. /// - /// The value of this field is currently only used for `realloc`. - original_data_len: [u8; 4], + /// Currently, this value is only used for `realloc` to determine if the + /// account data length has changed from the original serialized length beyond + /// the maximum permitted data increase. + original_data_len: u32, /// Public key of the account key: Pubkey, @@ -70,22 +72,19 @@ pub(crate) struct Account { pub(crate) data_len: u64, } -// Convenience macro to get the original data length from the account – the value will -// be zero on first access. -macro_rules! get_original_data_len { - ( $self:expr ) => { - unsafe { *(&(*$self).original_data_len as *const _ as *const u32) as usize } - }; -} +/// Mask to indicate the original data length has been set. +/// +/// This takes advantage of the fact that the original data length will not +/// be greater than 10_000_000 bytes, so we can use the most significant bit +/// as a flag to indicate that the original data length has been set and lazily +/// initialize its value. +const SET_LEN_MASK: u32 = 1 << 31; -// Convenience macro to set the original data length in the account. -macro_rules! set_original_data_len { - ( $self:expr, $len:expr ) => { - unsafe { - *(&mut (*$self).original_data_len) = u32::to_le_bytes($len as u32); - } - }; -} +/// Mask to retrieve the original data length. +/// +/// This mask is used to retrieve the original data length from the `original_data_len` +/// by clearing the flag that indicates the original data length has been set. +const GET_LEN_MASK: u32 = !SET_LEN_MASK; /// Wrapper struct for an `Account`. /// @@ -329,10 +328,16 @@ impl AccountInfo { return Ok(()); } - let original_len = match get_original_data_len!(self.raw) { - len if len > 0 => len, - _ => { - set_original_data_len!(self.raw, current_len); + let original_len = { + let length = unsafe { (*self.raw).original_data_len }; + + if length & SET_LEN_MASK == SET_LEN_MASK { + (length & GET_LEN_MASK) as usize + } else { + // lazily initialize the original data length and sets the flag + unsafe { + (*self.raw).original_data_len = (current_len as u32) | SET_LEN_MASK; + } current_len } }; @@ -357,7 +362,7 @@ impl AccountInfo { if len_increase > 0 { unsafe { sol_memset_( - &mut data[original_len..] as *mut _ as *mut u8, + &mut data[current_len..] as *mut _ as *mut u8, 0, len_increase as u64, ); From 542de2faae2bc9af31032591e05bae732ba28961 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Sat, 26 Oct 2024 00:52:31 +0100 Subject: [PATCH 10/51] Add `checked_create_program_address` helper (#30) * Add unchecked helper * Fix lint * Add inline * Rename to checked * Cosmetics * Fix sol log params --- account-view/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 9a5d2edf0..cc662b705 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -139,6 +139,12 @@ impl AccountInfo { unsafe { (*self.raw).data_len as usize } } + /// Returns the lamports in the account. + #[inline(always)] + pub fn lamports(&self) -> u64 { + unsafe { (*self.raw).lamports } + } + /// Indicates whether the account data is empty. /// /// An account is considered empty if the data length is zero. From 405288fe5cbc3fc79303820b1f28323ad01261f6 Mon Sep 17 00:00:00 2001 From: Leonardo Donatacci <125566964+L0STE@users.noreply.github.com> Date: Wed, 20 Nov 2024 20:03:51 +0100 Subject: [PATCH 11/51] A close macro implementation for AccountInfo (#42) * Added close and based_close * added docs comments + wrapped up and tested both function * cargo clippy and fmt * added the new close and changed the name for * fixed and tested after comments --- account-view/src/lib.rs | 51 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index cc662b705..8ca912ad1 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,8 +1,7 @@ //! Data structures to represent account information. - use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull, slice::from_raw_parts_mut}; -use crate::{program_error::ProgramError, pubkey::Pubkey, syscalls::sol_memset_}; +use crate::{program_error::ProgramError, pubkey::Pubkey, syscalls::sol_memset_, ProgramResult}; /// Maximum number of bytes a program may add to an account during a /// single realloc. @@ -379,6 +378,54 @@ impl AccountInfo { Ok(()) } + /// Zero out the the account's data_len, lamports and owner fields, effectively + /// closing the account. + /// + /// This doesn't protect against future reinitialization of the account + /// since the account_data will need to be zeroed out as well. Or if the attacker + /// has access to the keypair of the account that we're trying to close, they can + /// just add the lenght, lamports and owner back before the data is wiped out from + /// the ledger. + /// + /// This works because the 48 bytes before the account data are: + /// - 8 bytes for the data_len + /// - 8 bytes for the lamports + /// - 32 bytes for the owner + pub fn close(&self) -> ProgramResult { + { + let _ = self.try_borrow_mut_data()?; + } + + unsafe { + self.close_unchecked(); + } + + Ok(()) + } + /// + /// # Safety + /// + /// This method makes assumptions about the layout and location of memory + /// referenced by `AccountInfo` fields. It should only be called for + /// instances of `AccountInfo` that were created by the runtime and received + /// in the `process_instruction` entrypoint of a program. + /// + /// This method is unsafe because it does not check if the account data is already + /// borrowed. It should only be called when the account is not being used. + #[inline(always)] + pub unsafe fn close_unchecked(&self) { + // Zero out the account owner. While the field is a `Pubkey`, it is quicker + // to zero the 32 bytes as 4 x `u64`s. + *(self.data_ptr().sub(48) as *mut u64) = 0u64; + *(self.data_ptr().sub(40) as *mut u64) = 0u64; + *(self.data_ptr().sub(32) as *mut u64) = 0u64; + *(self.data_ptr().sub(16) as *mut u64) = 0u64; + // Zero the account lamports. + (*self.raw).lamports = 0; + // Zero the account data length. + (*self.raw).data_len = 0; + } + /// Returns the memory address of the account data. fn data_ptr(&self) -> *mut u8 { unsafe { (self.raw as *const _ as *mut u8).add(core::mem::size_of::()) } From 69161328d1e4dc86211726c37c66832da51493d8 Mon Sep 17 00:00:00 2001 From: Leonardo Donatacci <125566964+L0STE@users.noreply.github.com> Date: Wed, 20 Nov 2024 22:19:51 +0100 Subject: [PATCH 12/51] Fixed Realloc Macro (#45) * Fixed compiler bitching about realloc * Added a better alterantive to the black_box * Fixed latest comments * deleted some line after the refactor --- account-view/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 8ca912ad1..c89a909f0 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,7 +1,10 @@ //! Data structures to represent account information. use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull, slice::from_raw_parts_mut}; -use crate::{program_error::ProgramError, pubkey::Pubkey, syscalls::sol_memset_, ProgramResult}; +#[cfg(target_os = "solana")] +use crate::syscalls::sol_memset_; + +use crate::{program_error::ProgramError, pubkey::Pubkey, ProgramResult}; /// Maximum number of bytes a program may add to an account during a /// single realloc. @@ -366,11 +369,14 @@ impl AccountInfo { let len_increase = new_len.saturating_sub(current_len); if len_increase > 0 { unsafe { + #[cfg(target_os = "solana")] sol_memset_( &mut data[current_len..] as *mut _ as *mut u8, 0, len_increase as u64, ); + #[cfg(not(target_os = "solana"))] + core::ptr::write_bytes(data.as_mut_ptr().add(current_len), 0, len_increase); } } } From a58d65e38c6bc9cafec7da968b40d483ee714c60 Mon Sep 17 00:00:00 2001 From: febo Date: Fri, 22 Nov 2024 09:59:24 +0000 Subject: [PATCH 13/51] Update comments --- account-view/src/lib.rs | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index c89a909f0..1cc47d341 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -313,7 +313,7 @@ impl AccountInfo { /// memory. /// /// Note: Account data can be increased within a single call by up to - /// `solana_program::entrypoint::MAX_PERMITTED_DATA_INCREASE` bytes. + /// [`MAX_PERMITTED_DATA_INCREASE`] bytes. /// /// Note: Memory used to grow is already zero-initialized upon program /// entrypoint and re-zeroing it wastes compute units. If within the same @@ -384,21 +384,17 @@ impl AccountInfo { Ok(()) } - /// Zero out the the account's data_len, lamports and owner fields, effectively + /// Zero out the the account's data length, lamports and owner fields, effectively /// closing the account. /// /// This doesn't protect against future reinitialization of the account - /// since the account_data will need to be zeroed out as well. Or if the attacker - /// has access to the keypair of the account that we're trying to close, they can - /// just add the lenght, lamports and owner back before the data is wiped out from - /// the ledger. - /// - /// This works because the 48 bytes before the account data are: - /// - 8 bytes for the data_len - /// - 8 bytes for the lamports - /// - 32 bytes for the owner + /// since the account data will need to be zeroed out as well; otherwise the lenght, + /// lamports and owner can be set again before the data is wiped out from + /// the ledger using the keypair of the account being close. pub fn close(&self) -> ProgramResult { { + // make sure the account is not borrowed since we are about to + // resize the data to zero let _ = self.try_borrow_mut_data()?; } @@ -408,18 +404,33 @@ impl AccountInfo { Ok(()) } + + /// Zero out the the account's data length, lamports and owner fields, effectively + /// closing the account. + /// + /// This doesn't protect against future reinitialization of the account + /// since the account data will need to be zeroed out as well; otherwise the lenght, + /// lamports and owner can be set again before the data is wiped out from + /// the ledger using the keypair of the account being close. /// /// # Safety /// - /// This method makes assumptions about the layout and location of memory + /// This method is unsafe because it does not check if the account data is already + /// borrowed. It should only be called when the account is not being used. + /// + /// It also makes assumptions about the layout and location of memory /// referenced by `AccountInfo` fields. It should only be called for /// instances of `AccountInfo` that were created by the runtime and received /// in the `process_instruction` entrypoint of a program. - /// - /// This method is unsafe because it does not check if the account data is already - /// borrowed. It should only be called when the account is not being used. #[inline(always)] pub unsafe fn close_unchecked(&self) { + // We take advantage that the 48 bytes before the account data are: + // - 32 bytes for the owner + // - 8 bytes for the lamports + // - 8 bytes for the data_len + // + // So we can zero out them directly. + // Zero out the account owner. While the field is a `Pubkey`, it is quicker // to zero the 32 bytes as 4 x `u64`s. *(self.data_ptr().sub(48) as *mut u64) = 0u64; From 9889881c53a9105d2f4e7e9934e8b94ab35e2784 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Sun, 12 Jan 2025 23:53:20 +0000 Subject: [PATCH 14/51] Fix last u64 owner index on close (#55) --- account-view/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 1cc47d341..ef1338de5 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -436,7 +436,7 @@ impl AccountInfo { *(self.data_ptr().sub(48) as *mut u64) = 0u64; *(self.data_ptr().sub(40) as *mut u64) = 0u64; *(self.data_ptr().sub(32) as *mut u64) = 0u64; - *(self.data_ptr().sub(16) as *mut u64) = 0u64; + *(self.data_ptr().sub(24) as *mut u64) = 0u64; // Zero the account lamports. (*self.raw).lamports = 0; // Zero the account data length. From 8ce802eb4267e6c0cc80ef2055bcebd3b0550cd1 Mon Sep 17 00:00:00 2001 From: Dimitris Apostolou Date: Wed, 5 Feb 2025 12:26:01 +0200 Subject: [PATCH 15/51] Fix clippy warnings (#59) --- account-view/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index ef1338de5..a59f03c10 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -504,14 +504,14 @@ impl<'a, T: ?Sized> Ref<'a, T> { } } -impl<'a, T: ?Sized> core::ops::Deref for Ref<'a, T> { +impl core::ops::Deref for Ref<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { unsafe { self.value.as_ref() } } } -impl<'a, T: ?Sized> Drop for Ref<'a, T> { +impl Drop for Ref<'_, T> { // decrement the immutable borrow count fn drop(&mut self) { unsafe { *self.state.as_mut() -= 1 << self.borrow_shift }; @@ -576,19 +576,19 @@ impl<'a, T: ?Sized> RefMut<'a, T> { } } -impl<'a, T: ?Sized> core::ops::Deref for RefMut<'a, T> { +impl core::ops::Deref for RefMut<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { unsafe { self.value.as_ref() } } } -impl<'a, T: ?Sized> core::ops::DerefMut for RefMut<'a, T> { +impl core::ops::DerefMut for RefMut<'_, T> { fn deref_mut(&mut self) -> &mut ::Target { unsafe { self.value.as_mut() } } } -impl<'a, T: ?Sized> Drop for RefMut<'a, T> { +impl Drop for RefMut<'_, T> { // unset the mutable borrow flag fn drop(&mut self) { unsafe { *self.state.as_mut() &= self.borrow_mask }; From 5865a994c76f3fc8346863575fa63a4565c82e55 Mon Sep 17 00:00:00 2001 From: febo Date: Fri, 14 Feb 2025 00:02:15 +0000 Subject: [PATCH 16/51] Improve close unchecked --- account-view/src/lib.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index a59f03c10..6766b14c1 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -157,6 +157,7 @@ impl AccountInfo { /// Changes the owner of the account. #[allow(invalid_reference_casting)] + #[inline(always)] pub fn assign(&self, new_owner: &Pubkey) { unsafe { core::ptr::write_volatile(&(*self.raw).owner as *const _ as *mut Pubkey, *new_owner); @@ -169,6 +170,7 @@ impl AccountInfo { /// /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow /// flag untouched. Useful when an instruction has verified non-duplicate accounts. + #[inline(always)] pub unsafe fn borrow_lamports_unchecked(&self) -> &u64 { &(*self.raw).lamports } @@ -180,6 +182,7 @@ impl AccountInfo { /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[allow(clippy::mut_from_ref)] + #[inline(always)] pub unsafe fn borrow_mut_lamports_unchecked(&self) -> &mut u64 { &mut (*self.raw).lamports } @@ -190,6 +193,7 @@ impl AccountInfo { /// /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow /// flag untouched. Useful when an instruction has verified non-duplicate accounts. + #[inline(always)] pub unsafe fn borrow_data_unchecked(&self) -> &[u8] { core::slice::from_raw_parts(self.data_ptr(), self.data_len()) } @@ -201,6 +205,7 @@ impl AccountInfo { /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[allow(clippy::mut_from_ref)] + #[inline(always)] pub unsafe fn borrow_mut_data_unchecked(&self) -> &mut [u8] { core::slice::from_raw_parts_mut(self.data_ptr(), self.data_len()) } @@ -391,6 +396,7 @@ impl AccountInfo { /// since the account data will need to be zeroed out as well; otherwise the lenght, /// lamports and owner can be set again before the data is wiped out from /// the ledger using the keypair of the account being close. + #[inline] pub fn close(&self) -> ProgramResult { { // make sure the account is not borrowed since we are about to @@ -430,17 +436,8 @@ impl AccountInfo { // - 8 bytes for the data_len // // So we can zero out them directly. - - // Zero out the account owner. While the field is a `Pubkey`, it is quicker - // to zero the 32 bytes as 4 x `u64`s. - *(self.data_ptr().sub(48) as *mut u64) = 0u64; - *(self.data_ptr().sub(40) as *mut u64) = 0u64; - *(self.data_ptr().sub(32) as *mut u64) = 0u64; - *(self.data_ptr().sub(24) as *mut u64) = 0u64; - // Zero the account lamports. - (*self.raw).lamports = 0; - // Zero the account data length. - (*self.raw).data_len = 0; + #[cfg(target_os = "solana")] + sol_memset_(self.data_ptr().sub(48), 0, 48); } /// Returns the memory address of the account data. From 6b509bb45a958a5945a4b468d3b4cd601bff7baf Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 14 Feb 2025 20:43:05 +0000 Subject: [PATCH 17/51] sdk: Improve comments (#64) * [wip]: Add new scripts * [wip]: Use matric strategy * [wip]: Fix members parsing * [wip]: Add CI env variables * [wip]: Remove nothrow * [wip]: Filter changes * [wip]: Add audit step * [wip]: Add semver checks * [wip]: Refactor publish workflow * [wip]: Refactor * [wip]: Fix commands * Fix formatting * Remove detect changes step * Review comments * Fix lint comments * Expand crate comment * Ignore crate comment tests * Add missing docs * More missing docs * Add missing release component * Pin cargo-release version * Fix merge * Review comments --- account-view/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 6766b14c1..154c9215d 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -10,6 +10,9 @@ use crate::{program_error::ProgramError, pubkey::Pubkey, ProgramResult}; /// single realloc. pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; +/// Returns the account info at the given index. +/// +/// This macro validates that the index is within the bounds of the accounts. #[macro_export] macro_rules! get_account_info { ( $accounts:ident, $index:expr ) => {{ @@ -465,6 +468,7 @@ pub struct Ref<'a, T: ?Sized> { } impl<'a, T: ?Sized> Ref<'a, T> { + /// Maps a reference to a new type. #[inline] pub fn map(orig: Ref<'a, T>, f: F) -> Ref<'a, U> where @@ -481,6 +485,7 @@ impl<'a, T: ?Sized> Ref<'a, T> { } } + /// Filters and maps a reference to a new type. #[inline] pub fn filter_map(orig: Ref<'a, T>, f: F) -> Result, Self> where @@ -534,6 +539,7 @@ pub struct RefMut<'a, T: ?Sized> { } impl<'a, T: ?Sized> RefMut<'a, T> { + /// Maps a mutable reference to a new type. #[inline] pub fn map(orig: RefMut<'a, T>, f: F) -> RefMut<'a, U> where @@ -550,6 +556,7 @@ impl<'a, T: ?Sized> RefMut<'a, T> { } } + /// Filters and maps a mutable reference to a new type. #[inline] pub fn filter_map(orig: RefMut<'a, T>, f: F) -> Result, Self> where @@ -586,8 +593,8 @@ impl core::ops::DerefMut for RefMut<'_, T> { } impl Drop for RefMut<'_, T> { - // unset the mutable borrow flag fn drop(&mut self) { + // unset the mutable borrow flag unsafe { *self.state.as_mut() &= self.borrow_mask }; } } From b558400f2979412d71f9701fde28060a306dae71 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Sat, 15 Feb 2025 01:22:03 +0000 Subject: [PATCH 18/51] sdk: Lightweight borrow check (#65) * [wip]: Add new scripts * [wip]: Add CI env variables * [wip]: Remove nothrow * [wip]: Filter changes * [wip]: Add audit step * [wip]: Add semver checks * [wip]: Refactor publish workflow * [wip]: Refactor * [wip]: Fix commands * Fix formatting * Remove detect changes step * Add check methods * Use check variant on close * Fix merge --- account-view/src/lib.rs | 132 ++++++++++++++++++++++++++-------------- 1 file changed, 87 insertions(+), 45 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 154c9215d..beb0056e9 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,5 +1,10 @@ //! Data structures to represent account information. -use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull, slice::from_raw_parts_mut}; +use core::{ + marker::PhantomData, + mem::ManuallyDrop, + ptr::NonNull, + slice::{from_raw_parts, from_raw_parts_mut}, +}; #[cfg(target_os = "solana")] use crate::syscalls::sol_memset_; @@ -216,18 +221,10 @@ impl AccountInfo { /// Tries to get a read-only reference to the lamport field, failing if the /// field is already mutable borrowed or if 7 borrows already exist. pub fn try_borrow_lamports(&self) -> Result, ProgramError> { - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - - // check if mutable borrow is already taken - if *borrow_state & 0b_1000_0000 != 0 { - return Err(ProgramError::AccountBorrowFailed); - } - - // check if we have reached the max immutable borrow count - if *borrow_state & 0b_0111_0000 == 0b_0111_0000 { - return Err(ProgramError::AccountBorrowFailed); - } + // check if the account lamports are already borrowed + self.check_borrow_lamports()?; + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // increment the immutable borrow count *borrow_state += 1 << LAMPORTS_SHIFT; @@ -243,13 +240,10 @@ impl AccountInfo { /// Tries to get a read only reference to the lamport field, failing if the field /// is already borrowed in any form. pub fn try_borrow_mut_lamports(&self) -> Result, ProgramError> { - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - - // check if any borrow (mutable or immutable) is already taken for lamports - if *borrow_state & 0b_1111_0000 != 0 { - return Err(ProgramError::AccountBorrowFailed); - } + // check if the account lamports are already borrowed + self.check_borrow_mut_lamports()?; + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // set the mutable lamport borrow flag *borrow_state |= 0b_1000_0000; @@ -262,49 +256,65 @@ impl AccountInfo { }) } - /// Tries to get a read only reference to the data field, failing if the field - /// is already mutable borrowed or if 7 borrows already exist. - pub fn try_borrow_data(&self) -> Result, ProgramError> { - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; + /// Checks if it is possible to get a read-only reference to the lamport field, + /// failing if the field is already mutable borrowed or if 7 borrows already exist. + #[inline(always)] + pub fn check_borrow_lamports(&self) -> Result<(), ProgramError> { + let borrow_state = unsafe { (*self.raw).borrow_state }; - // check if mutable data borrow is already taken (most significant bit - // of the data_borrow_state) - if *borrow_state & 0b_0000_1000 != 0 { + // check if mutable borrow is already taken + if borrow_state & 0b_1000_0000 != 0 { return Err(ProgramError::AccountBorrowFailed); } - // check if we have reached the max immutable data borrow count (7) - if *borrow_state & 0b_0111 == 0b0111 { + // check if we have reached the max immutable borrow count + if borrow_state & 0b_0111_0000 == 0b_0111_0000 { + return Err(ProgramError::AccountBorrowFailed); + } + + Ok(()) + } + + /// Checks if it is possible to get a mutable reference to the lamport field, + /// failing if the field is already borrowed in any form. + #[inline(always)] + pub fn check_borrow_mut_lamports(&self) -> Result<(), ProgramError> { + let borrow_state = unsafe { (*self.raw).borrow_state }; + + // check if any borrow (mutable or immutable) is already taken for lamports + if borrow_state & 0b_1111_0000 != 0 { return Err(ProgramError::AccountBorrowFailed); } + Ok(()) + } + + /// Tries to get a read-only reference to the data field, failing if the field + /// is already mutable borrowed or if 7 borrows already exist. + pub fn try_borrow_data(&self) -> Result, ProgramError> { + // check if the account data is already borrowed + self.check_borrow_data()?; + + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // increment the immutable data borrow count *borrow_state += 1; // return the reference to data Ok(Ref { - value: unsafe { - NonNull::from(core::slice::from_raw_parts( - self.data_ptr(), - self.data_len(), - )) - }, + value: unsafe { NonNull::from(from_raw_parts(self.data_ptr(), self.data_len())) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, borrow_shift: DATA_SHIFT, marker: PhantomData, }) } - /// Tries to get a read only reference to the data field, failing if the field + /// Tries to get a mutable reference to the data field, failing if the field /// is already borrowed in any form. pub fn try_borrow_mut_data(&self) -> Result, ProgramError> { - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - - // check if any borrow (mutable or immutable) is already taken for data - if *borrow_state & 0b_0000_1111 != 0 { - return Err(ProgramError::AccountBorrowFailed); - } + // check if the account data is already borrowed + self.check_borrow_mut_data()?; + let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // set the mutable data borrow flag *borrow_state |= 0b_0000_1000; @@ -317,6 +327,40 @@ impl AccountInfo { }) } + /// Checks if it is possible to get a read-only reference to the data field, failing + /// if the field is already mutable borrowed or if 7 borrows already exist. + #[inline(always)] + pub fn check_borrow_data(&self) -> Result<(), ProgramError> { + let borrow_state = unsafe { (*self.raw).borrow_state }; + + // check if mutable data borrow is already taken (most significant bit + // of the data_borrow_state) + if borrow_state & 0b_0000_1000 != 0 { + return Err(ProgramError::AccountBorrowFailed); + } + + // check if we have reached the max immutable data borrow count (7) + if borrow_state & 0b_0111 == 0b0111 { + return Err(ProgramError::AccountBorrowFailed); + } + + Ok(()) + } + + /// Checks if it is possible to get a mutable reference to the data field, failing + /// if the field is already borrowed in any form. + #[inline(always)] + pub fn check_borrow_mut_data(&self) -> Result<(), ProgramError> { + let borrow_state = unsafe { (*self.raw).borrow_state }; + + // check if any borrow (mutable or immutable) is already taken for data + if borrow_state & 0b_0000_1111 != 0 { + return Err(ProgramError::AccountBorrowFailed); + } + + Ok(()) + } + /// Realloc the account's data and optionally zero-initialize the new /// memory. /// @@ -401,11 +445,9 @@ impl AccountInfo { /// the ledger using the keypair of the account being close. #[inline] pub fn close(&self) -> ProgramResult { - { - // make sure the account is not borrowed since we are about to - // resize the data to zero - let _ = self.try_borrow_mut_data()?; - } + // make sure the account is not borrowed since we are about to + // resize the data to zero + self.check_borrow_mut_data()?; unsafe { self.close_unchecked(); From 0fb5a399da268414f6749f950c0755fd55358d16 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 14 Mar 2025 20:51:02 +0000 Subject: [PATCH 19/51] Address review comments (#78) * [wip]: Address review comments * [wip]: Fix pointer reference * [wip]: Add logger buffer size tests * Remove unused * More logger tests * Rename program to cpi * Remove dynamic allocation * Fixed signed tests * Fix review comments * Fix unsigned test case * Add is_owner_by helper --- account-view/src/lib.rs | 66 ++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index beb0056e9..e9ca62c63 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -12,22 +12,9 @@ use crate::syscalls::sol_memset_; use crate::{program_error::ProgramError, pubkey::Pubkey, ProgramResult}; /// Maximum number of bytes a program may add to an account during a -/// single realloc. +/// single top-level instruction. pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; -/// Returns the account info at the given index. -/// -/// This macro validates that the index is within the bounds of the accounts. -#[macro_export] -macro_rules! get_account_info { - ( $accounts:ident, $index:expr ) => {{ - if $accounts.len() <= $index { - return Err($crate::program_error::ProgramError::NotEnoughAccountKeys); - } - &$accounts[$index] - }}; -} - /// Raw account data. /// /// This data is wrapped in an `AccountInfo` struct, which provides safe access @@ -69,16 +56,16 @@ pub(crate) struct Account { /// the maximum permitted data increase. original_data_len: u32, - /// Public key of the account + /// Public key of the account. key: Pubkey, - /// Program that owns this account + /// Program that owns this account. Modifiable by programs. owner: Pubkey, - /// The lamports in the account. Modifiable by programs. + /// The lamports in the account. Modifiable by programs. lamports: u64, - /// Length of the data. + /// Length of the data. Modifiable by programs. pub(crate) data_len: u64, } @@ -118,9 +105,14 @@ impl AccountInfo { } /// Program that owns this account. + /// + /// # Safety + /// + /// A reference returned by this method is invalidated when [`Self::assign`] + /// is called. #[inline(always)] - pub fn owner(&self) -> &Pubkey { - unsafe { &(*self.raw).owner } + pub unsafe fn owner(&self) -> &Pubkey { + &(*self.raw).owner } /// Indicates whether the transaction was signed by this account. @@ -163,13 +155,21 @@ impl AccountInfo { self.data_len() == 0 } + /// Checks if the account is owned by the given program. + #[inline(always)] + pub fn is_owned_by(&self, program: &Pubkey) -> bool { + unsafe { &(*self.raw).owner == program } + } + /// Changes the owner of the account. - #[allow(invalid_reference_casting)] + /// + /// # Safety + /// + /// Using this method invalidates any reference returned by [`Self::owner`]. #[inline(always)] - pub fn assign(&self, new_owner: &Pubkey) { - unsafe { - core::ptr::write_volatile(&(*self.raw).owner as *const _ as *mut Pubkey, *new_owner); - } + pub unsafe fn assign(&self, new_owner: &Pubkey) { + #[allow(invalid_reference_casting)] + core::ptr::write_volatile(&(*self.raw).owner as *const _ as *mut Pubkey, *new_owner); } /// Returns a read-only reference to the lamports in the account. @@ -412,7 +412,7 @@ impl AccountInfo { unsafe { let data_ptr = data.as_mut_ptr(); // set new length in the serialized data - *(data_ptr.offset(-8) as *mut u64) = new_len as u64; + (*self.raw).data_len = new_len as u64; // recreate the local slice with the new length data.value = NonNull::from(from_raw_parts_mut(data_ptr, new_len)); } @@ -442,7 +442,12 @@ impl AccountInfo { /// This doesn't protect against future reinitialization of the account /// since the account data will need to be zeroed out as well; otherwise the lenght, /// lamports and owner can be set again before the data is wiped out from - /// the ledger using the keypair of the account being close. + /// the ledger using the keypair of the account being closed. + /// + /// # Important + /// + /// The lamports must be moved from the account prior to closing it to prevent + /// an unbalanced instruction error. #[inline] pub fn close(&self) -> ProgramResult { // make sure the account is not borrowed since we are about to @@ -462,7 +467,12 @@ impl AccountInfo { /// This doesn't protect against future reinitialization of the account /// since the account data will need to be zeroed out as well; otherwise the lenght, /// lamports and owner can be set again before the data is wiped out from - /// the ledger using the keypair of the account being close. + /// the ledger using the keypair of the account being closed. + /// + /// # Important + /// + /// The lamports must be moved from the account prior to closing it to prevent + /// an unbalanced instruction error. /// /// # Safety /// From cef800da0163f41bf7a0b07f7ad488cbcd52f939 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Thu, 1 May 2025 12:30:43 +0100 Subject: [PATCH 20/51] Account borrow state check (#147) * Improve fallback and docs * Add borrow state check * Add inline * Review comments * Revert doc link merge change --- account-view/src/lib.rs | 69 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index e9ca62c63..9b0d22f16 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -15,6 +15,21 @@ use crate::{program_error::ProgramError, pubkey::Pubkey, ProgramResult}; /// single top-level instruction. pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; +/// Represents masks for borrow state of an account. +#[repr(u8)] +#[derive(Clone, Copy)] +pub enum BorrowState { + /// Mask to check whether an account is already borrowed. + /// + /// This will test both data and lamports borrow state. + Borrowed = 0b_1111_1111, + + /// Mask to check whether an account is already mutably borrowed. + /// + /// This will test both data and lamports mutable borrow state. + MutablyBorrowed = 0b_1000_1000, +} + /// Raw account data. /// /// This data is wrapped in an `AccountInfo` struct, which provides safe access @@ -172,6 +187,15 @@ impl AccountInfo { core::ptr::write_volatile(&(*self.raw).owner as *const _ as *mut Pubkey, *new_owner); } + /// Return true if the account borrow state is set to the given state. + /// + /// This will test both data and lamports borrow state. + #[inline(always)] + pub fn is_borrowed(&self, state: BorrowState) -> bool { + let borrow_state = unsafe { (*self.raw).borrow_state }; + borrow_state & (state as u8) != 0 + } + /// Returns a read-only reference to the lamports in the account. /// /// # Safety @@ -222,7 +246,7 @@ impl AccountInfo { /// field is already mutable borrowed or if 7 borrows already exist. pub fn try_borrow_lamports(&self) -> Result, ProgramError> { // check if the account lamports are already borrowed - self.check_borrow_lamports()?; + self.can_borrow_lamports()?; let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // increment the immutable borrow count @@ -241,7 +265,7 @@ impl AccountInfo { /// is already borrowed in any form. pub fn try_borrow_mut_lamports(&self) -> Result, ProgramError> { // check if the account lamports are already borrowed - self.check_borrow_mut_lamports()?; + self.can_borrow_mut_lamports()?; let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // set the mutable lamport borrow flag @@ -258,8 +282,16 @@ impl AccountInfo { /// Checks if it is possible to get a read-only reference to the lamport field, /// failing if the field is already mutable borrowed or if 7 borrows already exist. + #[deprecated(since = "0.8.4", note = "Use `can_borrow_lamports` instead")] #[inline(always)] pub fn check_borrow_lamports(&self) -> Result<(), ProgramError> { + self.can_borrow_lamports() + } + + /// Checks if it is possible to get a read-only reference to the lamport field, + /// failing if the field is already mutable borrowed or if 7 borrows already exist. + #[inline(always)] + pub fn can_borrow_lamports(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if mutable borrow is already taken @@ -277,8 +309,16 @@ impl AccountInfo { /// Checks if it is possible to get a mutable reference to the lamport field, /// failing if the field is already borrowed in any form. + #[deprecated(since = "0.8.4", note = "Use `can_borrow_mut_lamports` instead")] #[inline(always)] pub fn check_borrow_mut_lamports(&self) -> Result<(), ProgramError> { + self.can_borrow_mut_lamports() + } + + /// Checks if it is possible to get a mutable reference to the lamport field, + /// failing if the field is already borrowed in any form. + #[inline(always)] + pub fn can_borrow_mut_lamports(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if any borrow (mutable or immutable) is already taken for lamports @@ -293,7 +333,7 @@ impl AccountInfo { /// is already mutable borrowed or if 7 borrows already exist. pub fn try_borrow_data(&self) -> Result, ProgramError> { // check if the account data is already borrowed - self.check_borrow_data()?; + self.can_borrow_data()?; let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // increment the immutable data borrow count @@ -312,7 +352,7 @@ impl AccountInfo { /// is already borrowed in any form. pub fn try_borrow_mut_data(&self) -> Result, ProgramError> { // check if the account data is already borrowed - self.check_borrow_mut_data()?; + self.can_borrow_mut_data()?; let borrow_state = unsafe { &mut (*self.raw).borrow_state }; // set the mutable data borrow flag @@ -329,8 +369,16 @@ impl AccountInfo { /// Checks if it is possible to get a read-only reference to the data field, failing /// if the field is already mutable borrowed or if 7 borrows already exist. + #[deprecated(since = "0.8.4", note = "Use `can_borrow_data` instead")] #[inline(always)] pub fn check_borrow_data(&self) -> Result<(), ProgramError> { + self.can_borrow_data() + } + + /// Checks if it is possible to get a read-only reference to the data field, failing + /// if the field is already mutable borrowed or if 7 borrows already exist. + #[inline(always)] + pub fn can_borrow_data(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if mutable data borrow is already taken (most significant bit @@ -349,8 +397,16 @@ impl AccountInfo { /// Checks if it is possible to get a mutable reference to the data field, failing /// if the field is already borrowed in any form. + #[deprecated(since = "0.8.4", note = "Use `can_borrow_mut_data` instead")] #[inline(always)] pub fn check_borrow_mut_data(&self) -> Result<(), ProgramError> { + self.can_borrow_mut_data() + } + + /// Checks if it is possible to get a mutable reference to the data field, failing + /// if the field is already borrowed in any form. + #[inline(always)] + pub fn can_borrow_mut_data(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if any borrow (mutable or immutable) is already taken for data @@ -452,8 +508,11 @@ impl AccountInfo { pub fn close(&self) -> ProgramResult { // make sure the account is not borrowed since we are about to // resize the data to zero - self.check_borrow_mut_data()?; + if self.is_borrowed(BorrowState::Borrowed) { + return Err(ProgramError::AccountBorrowFailed); + } + // SAFETY: The are no active borrows on the account data or lamports. unsafe { self.close_unchecked(); } From 671b4564909d9b5ceb0ac754e633a4092b7349b4 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 6 Jun 2025 12:24:38 +0100 Subject: [PATCH 21/51] Update doc comments on close account (#173) * Update doc comments * Update sdk/pinocchio/src/account_info.rs Co-authored-by: Jon C * Update sdk/pinocchio/src/account_info.rs Co-authored-by: Jon C --------- Co-authored-by: Jon C --- account-view/src/lib.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 9b0d22f16..583a27109 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -495,10 +495,9 @@ impl AccountInfo { /// Zero out the the account's data length, lamports and owner fields, effectively /// closing the account. /// - /// This doesn't protect against future reinitialization of the account - /// since the account data will need to be zeroed out as well; otherwise the lenght, - /// lamports and owner can be set again before the data is wiped out from - /// the ledger using the keypair of the account being closed. + /// Note: This does not zero the account data. The account data will be zeroed by + /// the runtime at the end of the instruction where the account was closed or at the + /// next CPI call. /// /// # Important /// @@ -523,10 +522,9 @@ impl AccountInfo { /// Zero out the the account's data length, lamports and owner fields, effectively /// closing the account. /// - /// This doesn't protect against future reinitialization of the account - /// since the account data will need to be zeroed out as well; otherwise the lenght, - /// lamports and owner can be set again before the data is wiped out from - /// the ledger using the keypair of the account being closed. + /// Note: This does not zero the account data. The account data will be zeroed by + /// the runtime at the end of the instruction where the account was closed or at the + /// next CPI call. /// /// # Important /// From f56a65d9f82cda77b3cf9221f646c2b049383223 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 6 Jun 2025 12:38:21 +0100 Subject: [PATCH 22/51] Remove *const cast (#170) --- account-view/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 583a27109..2a28baae3 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -554,7 +554,7 @@ impl AccountInfo { /// Returns the memory address of the account data. fn data_ptr(&self) -> *mut u8 { - unsafe { (self.raw as *const _ as *mut u8).add(core::mem::size_of::()) } + unsafe { (self.raw as *mut u8).add(core::mem::size_of::()) } } } From b49d469c713bf6c8d17f0cfe74322e659a32f346 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Thu, 12 Jun 2025 01:04:59 +0100 Subject: [PATCH 23/51] Add cargo miri test to CI (#178) * Add miri step * Fix miri issues * Install miri component --- account-view/src/lib.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 2a28baae3..811631f72 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -715,12 +715,13 @@ mod tests { #[test] fn test_data_ref() { let data: [u8; 4] = [0, 1, 2, 3]; - let state = 1 << DATA_SHIFT; + let mut state = 1 << DATA_SHIFT; let ref_data = Ref { value: NonNull::from(&data), borrow_shift: DATA_SHIFT, - state: NonNull::from(&state), + // borrow state must be a mutable reference + state: NonNull::from(&mut state), marker: PhantomData, }; @@ -749,12 +750,13 @@ mod tests { #[test] fn test_lamports_ref() { let lamports: u64 = 10000; - let state = 1 << LAMPORTS_SHIFT; + let mut state = 1 << LAMPORTS_SHIFT; let ref_lamports = Ref { value: NonNull::from(&lamports), borrow_shift: LAMPORTS_SHIFT, - state: NonNull::from(&state), + // borrow state must be a mutable reference + state: NonNull::from(&mut state), marker: PhantomData, }; @@ -782,13 +784,14 @@ mod tests { #[test] fn test_data_ref_mut() { - let data: [u8; 4] = [0, 1, 2, 3]; - let state = 0b_0000_1000; + let mut data: [u8; 4] = [0, 1, 2, 3]; + let mut state = 0b_0000_1000; let ref_data = RefMut { - value: NonNull::from(&data), + value: NonNull::from(&mut data), borrow_mask: DATA_MASK, - state: NonNull::from(&state), + // borrow state must be a mutable reference + state: NonNull::from(&mut state), marker: PhantomData, }; @@ -809,13 +812,14 @@ mod tests { #[test] fn test_lamports_ref_mut() { - let lamports: u64 = 10000; - let state = 0b_1000_0000; + let mut lamports: u64 = 10000; + let mut state = 0b_1000_0000; let ref_lamports = RefMut { - value: NonNull::from(&lamports), + value: NonNull::from(&mut lamports), borrow_mask: LAMPORTS_MASK, - state: NonNull::from(&state), + // borrow state must be a mutable reference + state: NonNull::from(&mut state), marker: PhantomData, }; From 16e76fb4f82d1e599406e942b6336f9108cd47fd Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 13 Jun 2025 11:28:07 +0100 Subject: [PATCH 24/51] Deprecate AccountInfo::realloc (#174) * Add resize * Deprecate realloc --- account-view/src/lib.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 811631f72..39bd1b250 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -435,6 +435,7 @@ impl AccountInfo { /// referenced by `AccountInfo` fields. It should only be called for /// instances of `AccountInfo` that were created by the runtime and received /// in the `process_instruction` entrypoint of a program. + #[deprecated(since = "0.9.0", note = "Use AccountInfo::resize() instead")] pub fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), ProgramError> { let mut data = self.try_borrow_mut_data()?; let current_len = data.len(); @@ -492,6 +493,23 @@ impl AccountInfo { Ok(()) } + /// Resize (either truncating or zero extending) the account's data. + /// + /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes + /// within an instruction. + /// + /// # Important + /// + /// This method makes assumptions about the layout and location of memory + /// referenced by `AccountInfo` fields. It should only be called for + /// instances of `AccountInfo` that were created by the runtime and received + /// in the `process_instruction` entrypoint of a program. + #[inline] + pub fn resize(&self, new_len: usize) -> Result<(), ProgramError> { + #[allow(deprecated)] + self.realloc(new_len, true) + } + /// Zero out the the account's data length, lamports and owner fields, effectively /// closing the account. /// From ff0f9b21bdc04ec5a439ad4a50ffd1c5434948dd Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Sat, 14 Jun 2025 01:13:09 +0100 Subject: [PATCH 25/51] Simplify program entrypoint (#166) * Fix review comments * Revert offset increment change * Add invoke instruction helper * Typos * Remove new helpers * Remove unused * Address review comments * Tweak inline attributes * Use invoke signed unchecked * Refactor inline * Renamed to with_bounds * Update docs * Revert change * Add constant length check * Simplify accounts deserialization * Invert borrow state logic * Use expr instead * Add missing import * Address review comments * Revert unnecessary repr * Fix rebase * Tweak docs * Fix doc reference * Fix miri errors * More review comments --- account-view/src/lib.rs | 275 ++++++++++++++++++++++++++++++++-------- 1 file changed, 225 insertions(+), 50 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 39bd1b250..85eb87235 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -37,16 +37,38 @@ pub enum BorrowState { #[repr(C)] #[derive(Clone, Copy, Default)] pub(crate) struct Account { - /// Borrow state of the account data. + /// Borrow state for lamports and account data. /// - /// 0) We reuse the duplicate flag for this. We set it to 0b0000_0000. - /// 1) We use the first four bits to track state of lamport borrow - /// 2) We use the second four bits to track state of data borrow + /// This reuses the memory reserved for the duplicate flag in the + /// account to track lamports and data borrows. It represents the + /// numbers of borrows available. /// - /// 4 bit state: [1 bit mutable borrow flag | u3 immmutable borrow flag] - /// This gives us up to 7 immutable borrows. Note that does not mean 7 - /// duplicate account infos, but rather 7 calls to borrow lamports or - /// borrow data across all duplicate account infos. + /// Bits in the borrow byte are used as follows: + /// + /// * lamport mutable borrow flag + /// - `7 6 5 4 3 2 1 0` + /// - `x . . . . . . .`: `1` -> the lamport field can be mutably borrowed; + /// `0` -> there is an outstanding mutable borrow for the lamports. + /// + /// * lamport immutable borrow count + /// - `7 6 5 4 3 2 1 0` + /// - `. x x x . . . .`: number of immutable borrows that can still be + /// allocated, for the lamports field. Ranges from 7 (`111`) to + /// 0 (`000`). + /// + /// * data mutable borrow flag + /// - `7 6 5 4 3 2 1 0` + /// - `. . . . x . . .`: `1` -> the account data can be mutably borrowed; + /// `0` -> there is an outstanding mutable borrow for the account data. + /// + /// * data immutable borrow count + /// - `7 6 5 4 3 2 1 0` + /// - `. . . . . x x x`: Number of immutable borrows that can still be + /// allocated, for the account data. Ranges from 7 (`111`) to 0 (`000`). + /// + /// Note that this values are shared across `AccountInfo`s over the + /// same account, e.g., in case of duplicated accounts, they share + /// the same borrow state. pub(crate) borrow_state: u8, /// Indicates whether the transaction was signed by this account. @@ -193,7 +215,10 @@ impl AccountInfo { #[inline(always)] pub fn is_borrowed(&self, state: BorrowState) -> bool { let borrow_state = unsafe { (*self.raw).borrow_state }; - borrow_state & (state as u8) != 0 + let mask = state as u8; + // If borrow state has any of the state bits of the mask not set, + // then the account is borrowed for that state. + (borrow_state & mask) != mask } /// Returns a read-only reference to the lamports in the account. @@ -248,9 +273,13 @@ impl AccountInfo { // check if the account lamports are already borrowed self.can_borrow_lamports()?; - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - // increment the immutable borrow count - *borrow_state += 1 << LAMPORTS_SHIFT; + let borrow_state = self.raw as *mut u8; + // SAFETY: The `borrow_state` is a mutable pointer to the borrow state + // of the account, which is guaranteed to be valid. + // + // "consumes" one immutable borrow for lamports; we are guaranteed + // that there is at least one immutable borrow available. + unsafe { *borrow_state -= 1 << LAMPORTS_SHIFT }; // return the reference to lamports Ok(Ref { @@ -267,9 +296,13 @@ impl AccountInfo { // check if the account lamports are already borrowed self.can_borrow_mut_lamports()?; - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - // set the mutable lamport borrow flag - *borrow_state |= 0b_1000_0000; + let borrow_state = self.raw as *mut u8; + // SAFETY: The `borrow_state` is a mutable pointer to the borrow state + // of the account, which is guaranteed to be valid. + // + // "consumes" the mutable borrow for lamports; we are guaranteed + // that lamports are not already borrowed in any form + unsafe { *borrow_state &= 0b_0111_1111 }; // return the mutable reference to lamports Ok(RefMut { @@ -289,18 +322,18 @@ impl AccountInfo { } /// Checks if it is possible to get a read-only reference to the lamport field, - /// failing if the field is already mutable borrowed or if 7 borrows already exist. + /// failing if the field is already mutable borrowed or if `7` borrows already exist. #[inline(always)] pub fn can_borrow_lamports(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if mutable borrow is already taken - if borrow_state & 0b_1000_0000 != 0 { + if borrow_state & 0b_1000_0000 != 0b_1000_0000 { return Err(ProgramError::AccountBorrowFailed); } // check if we have reached the max immutable borrow count - if borrow_state & 0b_0111_0000 == 0b_0111_0000 { + if borrow_state & 0b_0111_0000 == 0 { return Err(ProgramError::AccountBorrowFailed); } @@ -322,7 +355,7 @@ impl AccountInfo { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if any borrow (mutable or immutable) is already taken for lamports - if borrow_state & 0b_1111_0000 != 0 { + if borrow_state & 0b_1111_0000 != 0b_1111_0000 { return Err(ProgramError::AccountBorrowFailed); } @@ -330,14 +363,18 @@ impl AccountInfo { } /// Tries to get a read-only reference to the data field, failing if the field - /// is already mutable borrowed or if 7 borrows already exist. + /// is already mutable borrowed or if `7` borrows already exist. pub fn try_borrow_data(&self) -> Result, ProgramError> { // check if the account data is already borrowed self.can_borrow_data()?; - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - // increment the immutable data borrow count - *borrow_state += 1; + let borrow_state = self.raw as *mut u8; + // SAFETY: The `borrow_state` is a mutable pointer to the borrow state + // of the account, which is guaranteed to be valid. + // + // "consumes" one immutable borrow for data; we are guaranteed + // that there is at least one immutable borrow available + unsafe { *borrow_state -= 1 }; // return the reference to data Ok(Ref { @@ -354,9 +391,13 @@ impl AccountInfo { // check if the account data is already borrowed self.can_borrow_mut_data()?; - let borrow_state = unsafe { &mut (*self.raw).borrow_state }; - // set the mutable data borrow flag - *borrow_state |= 0b_0000_1000; + let borrow_state = self.raw as *mut u8; + // SAFETY: The `borrow_state` is a mutable pointer to the borrow state + // of the account, which is guaranteed to be valid. + // + // "consumes" the mutable borrow for account data; we are guaranteed + // that account data are not already borrowed in any form + unsafe { *borrow_state &= 0b_1111_0111 }; // return the mutable reference to data Ok(RefMut { @@ -382,13 +423,13 @@ impl AccountInfo { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if mutable data borrow is already taken (most significant bit - // of the data_borrow_state) - if borrow_state & 0b_0000_1000 != 0 { + // of the data borrow state) + if borrow_state & 0b_0000_1000 == 0 { return Err(ProgramError::AccountBorrowFailed); } // check if we have reached the max immutable data borrow count (7) - if borrow_state & 0b_0111 == 0b0111 { + if borrow_state & 0b_0000_0111 == 0 { return Err(ProgramError::AccountBorrowFailed); } @@ -410,7 +451,7 @@ impl AccountInfo { let borrow_state = unsafe { (*self.raw).borrow_state }; // check if any borrow (mutable or immutable) is already taken for data - if borrow_state & 0b_0000_1111 != 0 { + if borrow_state & 0b_0000_1111 != 0b_0000_1111 { return Err(ProgramError::AccountBorrowFailed); } @@ -643,15 +684,15 @@ impl core::ops::Deref for Ref<'_, T> { impl Drop for Ref<'_, T> { // decrement the immutable borrow count fn drop(&mut self) { - unsafe { *self.state.as_mut() -= 1 << self.borrow_shift }; + unsafe { *self.state.as_mut() += 1 << self.borrow_shift }; } } /// Mask representing the mutable borrow flag for lamports. -const LAMPORTS_MASK: u8 = 0b_0111_1111; +const LAMPORTS_MASK: u8 = 0b_1000_0000; /// Mask representing the mutable borrow flag for data. -const DATA_MASK: u8 = 0b_1111_0111; +const DATA_MASK: u8 = 0b_0000_1000; /// Mutable reference to account data or lamports with checked borrow rules. pub struct RefMut<'a, T: ?Sized> { @@ -722,18 +763,22 @@ impl core::ops::DerefMut for RefMut<'_, T> { impl Drop for RefMut<'_, T> { fn drop(&mut self) { // unset the mutable borrow flag - unsafe { *self.state.as_mut() &= self.borrow_mask }; + unsafe { *self.state.as_mut() |= self.borrow_mask }; } } #[cfg(test)] mod tests { + use core::mem::{size_of, MaybeUninit}; + + use crate::NON_DUP_MARKER as NOT_BORROWED; + use super::*; #[test] fn test_data_ref() { let data: [u8; 4] = [0, 1, 2, 3]; - let mut state = 1 << DATA_SHIFT; + let mut state = NOT_BORROWED - (1 << DATA_SHIFT); let ref_data = Ref { value: NonNull::from(&data), @@ -745,30 +790,30 @@ mod tests { let new_ref = Ref::map(ref_data, |data| &data[1]); - assert_eq!(state, 1 << DATA_SHIFT); + assert_eq!(state, NOT_BORROWED - (1 << DATA_SHIFT)); assert_eq!(*new_ref, 1); let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&3)) else { unreachable!() }; - assert_eq!(state, 1 << DATA_SHIFT); + assert_eq!(state, NOT_BORROWED - (1 << DATA_SHIFT)); assert_eq!(*new_ref, 3); let new_ref = Ref::filter_map(new_ref, |_| Option::<&u8>::None); - assert_eq!(state, 1 << DATA_SHIFT); + assert_eq!(state, NOT_BORROWED - (1 << DATA_SHIFT)); assert!(new_ref.is_err()); drop(new_ref); - assert_eq!(state, 0 << DATA_SHIFT); + assert_eq!(state, NOT_BORROWED); } #[test] fn test_lamports_ref() { let lamports: u64 = 10000; - let mut state = 1 << LAMPORTS_SHIFT; + let mut state = NOT_BORROWED - (1 << LAMPORTS_SHIFT); let ref_lamports = Ref { value: NonNull::from(&lamports), @@ -780,30 +825,30 @@ mod tests { let new_ref = Ref::map(ref_lamports, |_| &1000); - assert_eq!(state, 1 << LAMPORTS_SHIFT); + assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_SHIFT)); assert_eq!(*new_ref, 1000); let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&2000)) else { unreachable!() }; - assert_eq!(state, 1 << LAMPORTS_SHIFT); + assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_SHIFT)); assert_eq!(*new_ref, 2000); let new_ref = Ref::filter_map(new_ref, |_| Option::<&i32>::None); - assert_eq!(state, 1 << LAMPORTS_SHIFT); + assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_SHIFT)); assert!(new_ref.is_err()); drop(new_ref); - assert_eq!(state, 0 << LAMPORTS_SHIFT); + assert_eq!(state, NOT_BORROWED); } #[test] fn test_data_ref_mut() { let mut data: [u8; 4] = [0, 1, 2, 3]; - let mut state = 0b_0000_1000; + let mut state = 0b_1111_0111; let ref_data = RefMut { value: NonNull::from(&mut data), @@ -819,19 +864,19 @@ mod tests { *new_ref = 4; - assert_eq!(state, 8); + assert_eq!(state, 0b_1111_0111); assert_eq!(*new_ref, 4); drop(new_ref); assert_eq!(data, [4, 1, 2, 3]); - assert_eq!(state, 0); + assert_eq!(state, NOT_BORROWED); } #[test] fn test_lamports_ref_mut() { let mut lamports: u64 = 10000; - let mut state = 0b_1000_0000; + let mut state = 0b_0111_1111; let ref_lamports = RefMut { value: NonNull::from(&mut lamports), @@ -846,12 +891,142 @@ mod tests { lamports }); - assert_eq!(state, 128); + assert_eq!(state, 0b_0111_1111); assert_eq!(*new_ref, 200); drop(new_ref); assert_eq!(lamports, 200); - assert_eq!(state, 0); + assert_eq!(state, NOT_BORROWED); + } + + #[test] + fn test_borrow_data() { + // 8-bytes aligned account data. + let mut data = [0u64; size_of::() / size_of::()]; + // Set the borrow state. + data[0] = NOT_BORROWED as u64; + let account_info = AccountInfo { + raw: data.as_mut_ptr() as *mut Account, + }; + + // Check that we can borrow data and lamports. + assert!(account_info.can_borrow_data().is_ok()); + assert!(account_info.can_borrow_mut_data().is_ok()); + assert!(account_info.can_borrow_lamports().is_ok()); + assert!(account_info.can_borrow_mut_lamports().is_ok()); + + // Borrow immutable data (7 immutable borrows available). + const ACCOUNT_REF: MaybeUninit> = MaybeUninit::>::uninit(); + let mut refs = [ACCOUNT_REF; 7]; + + refs.iter_mut().for_each(|r| { + let Ok(data_ref) = account_info.try_borrow_data() else { + panic!("Failed to borrow data"); + }; + r.write(data_ref); + }); + + // Check that we cannot borrow the data anymore. + assert!(account_info.can_borrow_data().is_err()); + assert!(account_info.try_borrow_data().is_err()); + assert!(account_info.can_borrow_mut_data().is_err()); + assert!(account_info.try_borrow_mut_data().is_err()); + // Lamports should still be borrowable. + assert!(account_info.can_borrow_lamports().is_ok()); + assert!(account_info.can_borrow_mut_lamports().is_ok()); + + // Drop the immutable borrows. + refs.iter_mut().for_each(|r| { + let r = unsafe { r.assume_init_read() }; + drop(r); + }); + + // We should be able to borrow the data again. + assert!(account_info.can_borrow_data().is_ok()); + assert!(account_info.can_borrow_mut_data().is_ok()); + + // Borrow mutable data. + let ref_mut = account_info.try_borrow_mut_data().unwrap(); + + // Check that we cannot borrow the data anymore. + assert!(account_info.can_borrow_data().is_err()); + assert!(account_info.try_borrow_data().is_err()); + assert!(account_info.can_borrow_mut_data().is_err()); + assert!(account_info.try_borrow_mut_data().is_err()); + + drop(ref_mut); + + // We should be able to borrow the data again. + assert!(account_info.can_borrow_data().is_ok()); + assert!(account_info.can_borrow_mut_data().is_ok()); + + let borrow_state = unsafe { (*account_info.raw).borrow_state }; + assert!(borrow_state == NOT_BORROWED); + } + + #[test] + fn test_borrow_lamports() { + // 8-bytes aligned account data. + let mut data = [0u64; size_of::() / size_of::()]; + // Set the borrow state. + data[0] = NOT_BORROWED as u64; + let account_info = AccountInfo { + raw: data.as_mut_ptr() as *mut Account, + }; + + // Check that we can borrow lamports and data. + assert!(account_info.can_borrow_lamports().is_ok()); + assert!(account_info.can_borrow_mut_lamports().is_ok()); + assert!(account_info.can_borrow_data().is_ok()); + assert!(account_info.can_borrow_mut_data().is_ok()); + + // Borrow immutable lamports (7 immutable borrows available). + const LAMPORTS_REF: MaybeUninit> = MaybeUninit::>::uninit(); + let mut refs = [LAMPORTS_REF; 7]; + + refs.iter_mut().for_each(|r| { + let Ok(lamports_ref) = account_info.try_borrow_lamports() else { + panic!("Failed to borrow lamports"); + }; + r.write(lamports_ref); + }); + + // Check that we cannot borrow the lamports anymore. + assert!(account_info.can_borrow_lamports().is_err()); + assert!(account_info.try_borrow_lamports().is_err()); + assert!(account_info.can_borrow_mut_lamports().is_err()); + assert!(account_info.try_borrow_mut_lamports().is_err()); + // Data should still be borrowable. + assert!(account_info.can_borrow_data().is_ok()); + assert!(account_info.can_borrow_mut_data().is_ok()); + + // Drop the immutable borrows. + refs.iter_mut().for_each(|r| { + let r = unsafe { r.assume_init_read() }; + drop(r); + }); + + // We should be able to borrow the lamports again. + assert!(account_info.can_borrow_lamports().is_ok()); + assert!(account_info.can_borrow_mut_lamports().is_ok()); + + // Borrow mutable lamports. + let ref_mut = account_info.try_borrow_mut_lamports().unwrap(); + + // Check that we cannot borrow the lamports anymore. + assert!(account_info.can_borrow_lamports().is_err()); + assert!(account_info.try_borrow_lamports().is_err()); + assert!(account_info.can_borrow_mut_lamports().is_err()); + assert!(account_info.try_borrow_mut_lamports().is_err()); + + drop(ref_mut); + + // We should be able to borrow the data again. + assert!(account_info.can_borrow_lamports().is_ok()); + assert!(account_info.can_borrow_mut_lamports().is_ok()); + + let borrow_state = unsafe { (*account_info.raw).borrow_state }; + assert!(borrow_state == NOT_BORROWED); } } From 98d1a1ca427af42e38e8e1e3915da5c22fb17ecc Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 20 Jun 2025 13:40:19 +0100 Subject: [PATCH 26/51] Simplify realloc logic (#175) * Simplify realloc logic * Address review comments --- account-view/src/lib.rs | 188 ++++++++++++++++++++++++++-------------- 1 file changed, 122 insertions(+), 66 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 85eb87235..27ad372b6 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,4 +1,5 @@ //! Data structures to represent account information. + use core::{ marker::PhantomData, mem::ManuallyDrop, @@ -80,18 +81,13 @@ pub(crate) struct Account { /// Indicates whether this account represents a program. executable: u8, - /// Account's original data length when it was serialized for the - /// current program invocation. - /// - /// The value of this field is lazily initialized to the current data length - /// and the [`SET_LEN_MASK`] flag on first access. When reading this field, - /// the flag is cleared to retrieve the original data length by using the - /// [`GET_LEN_MASK`] mask. + /// Difference between the original data length and the current + /// data length. /// - /// Currently, this value is only used for `realloc` to determine if the - /// account data length has changed from the original serialized length beyond - /// the maximum permitted data increase. - original_data_len: u32, + /// This is used to track the original data length of the account + /// when the account is resized. The runtime guarantees that this + /// value is zero at the start of the instruction. + resize_delta: i32, /// Public key of the account. key: Pubkey, @@ -106,20 +102,6 @@ pub(crate) struct Account { pub(crate) data_len: u64, } -/// Mask to indicate the original data length has been set. -/// -/// This takes advantage of the fact that the original data length will not -/// be greater than 10_000_000 bytes, so we can use the most significant bit -/// as a flag to indicate that the original data length has been set and lazily -/// initialize its value. -const SET_LEN_MASK: u32 = 1 << 31; - -/// Mask to retrieve the original data length. -/// -/// This mask is used to retrieve the original data length from the `original_data_len` -/// by clearing the flag that indicates the original data length has been set. -const GET_LEN_MASK: u32 = !SET_LEN_MASK; - /// Wrapper struct for an `Account`. /// /// This struct provides safe access to the data in an `Account`. It is also @@ -178,6 +160,16 @@ impl AccountInfo { unsafe { (*self.raw).data_len as usize } } + /// Returns the delta between the original data length and the current + /// data length. + /// + /// This value will be different than zero if the account has been resized + /// during the current instruction. + #[inline(always)] + pub fn resize_delta(&self) -> i32 { + unsafe { (*self.raw).resize_delta } + } + /// Returns the lamports in the account. #[inline(always)] pub fn lamports(&self) -> u64 { @@ -461,15 +453,21 @@ impl AccountInfo { /// Realloc the account's data and optionally zero-initialize the new /// memory. /// - /// Note: Account data can be increased within a single call by up to - /// [`MAX_PERMITTED_DATA_INCREASE`] bytes. + /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes + /// within an instruction. /// - /// Note: Memory used to grow is already zero-initialized upon program - /// entrypoint and re-zeroing it wastes compute units. If within the same + /// # Important + /// + /// Memory used to grow is already zero-initialized upon program + /// entrypoint and re-zeroing it wastes compute units. If within the same /// call a program reallocs from larger to smaller and back to larger again - /// the new space could contain stale data. Pass `true` for `zero_init` in + /// the new space could contain stale data. Pass `true` for `zero_init` in /// this case, otherwise compute units will be wasted re-zero-initializing. /// + /// Note that `zero_init` being `false` will not be supported by the + /// program runtime in the future. Programs should not rely on being able to + /// access stale data and use [`Self::resize`] instead. + /// /// # Safety /// /// This method makes assumptions about the layout and location of memory @@ -478,56 +476,47 @@ impl AccountInfo { /// in the `process_instruction` entrypoint of a program. #[deprecated(since = "0.9.0", note = "Use AccountInfo::resize() instead")] pub fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), ProgramError> { - let mut data = self.try_borrow_mut_data()?; - let current_len = data.len(); + // Check wheather the account data is already borrowed. + self.can_borrow_mut_data()?; + + // Account length is always `< i32::MAX`... + let current_len = self.data_len() as i32; + // ...so the new length must fit in an `i32`. + let new_len = i32::try_from(new_len).map_err(|_| ProgramError::InvalidRealloc)?; - // return early if length hasn't changed + // Return early if length hasn't changed. if new_len == current_len { return Ok(()); } - let original_len = { - let length = unsafe { (*self.raw).original_data_len }; - - if length & SET_LEN_MASK == SET_LEN_MASK { - (length & GET_LEN_MASK) as usize - } else { - // lazily initialize the original data length and sets the flag - unsafe { - (*self.raw).original_data_len = (current_len as u32) | SET_LEN_MASK; - } - current_len - } - }; + let difference = new_len - current_len; + let accumulated_resize_delta = self.resize_delta() + difference; - // return early if the length increase from the original serialized data + // Return an error when the length increase from the original serialized data // length is too large and would result in an out of bounds allocation - if new_len.saturating_sub(original_len) > MAX_PERMITTED_DATA_INCREASE { + if accumulated_resize_delta > MAX_PERMITTED_DATA_INCREASE as i32 { return Err(ProgramError::InvalidRealloc); } - // realloc unsafe { - let data_ptr = data.as_mut_ptr(); - // set new length in the serialized data (*self.raw).data_len = new_len as u64; - // recreate the local slice with the new length - data.value = NonNull::from(from_raw_parts_mut(data_ptr, new_len)); + (*self.raw).resize_delta = accumulated_resize_delta; } - if zero_init { - let len_increase = new_len.saturating_sub(current_len); - if len_increase > 0 { - unsafe { - #[cfg(target_os = "solana")] - sol_memset_( - &mut data[current_len..] as *mut _ as *mut u8, - 0, - len_increase as u64, - ); - #[cfg(not(target_os = "solana"))] - core::ptr::write_bytes(data.as_mut_ptr().add(current_len), 0, len_increase); - } + if zero_init && difference > 0 { + unsafe { + #[cfg(target_os = "solana")] + sol_memset_( + self.data_ptr().add(current_len as usize), + 0, + difference as u64, + ); + #[cfg(not(target_os = "solana"))] + core::ptr::write_bytes( + self.data_ptr().add(current_len as usize), + 0, + difference as usize, + ); } } @@ -572,6 +561,10 @@ impl AccountInfo { // SAFETY: The are no active borrows on the account data or lamports. unsafe { + // Update the resize delta since closing an account will set its data length + // to zero (account length is always `< i32::MAX`). + (*self.raw).resize_delta = self.resize_delta() - self.data_len() as i32; + self.close_unchecked(); } @@ -590,6 +583,11 @@ impl AccountInfo { /// The lamports must be moved from the account prior to closing it to prevent /// an unbalanced instruction error. /// + /// If [`Self::realloc`] or [`Self::resize`] are called after closing the account, + /// they might incorrectly return an error for going over the limit if the account + /// previously had space allocated since this method does not update the + /// [`Self::resize_delta`] value. + /// /// # Safety /// /// This method is unsafe because it does not check if the account data is already @@ -1029,4 +1027,62 @@ mod tests { let borrow_state = unsafe { (*account_info.raw).borrow_state }; assert!(borrow_state == NOT_BORROWED); } + + #[test] + #[allow(deprecated)] + fn test_realloc() { + // 8-bytes aligned account data. + let mut data = [0u64; 100 * size_of::()]; + + // Set the borrow state. + data[0] = NOT_BORROWED as u64; + // Set the initial data length to 100. + // - index `10` is equal to offset `10 * size_of::() = 80` bytes. + data[10] = 100; + + let account = AccountInfo { + raw: data.as_mut_ptr() as *const _ as *mut Account, + }; + + assert_eq!(account.data_len(), 100); + assert_eq!(account.resize_delta(), 0); + + // increase the size. + + account.realloc(200, false).unwrap(); + + assert_eq!(account.data_len(), 200); + assert_eq!(account.resize_delta(), 100); + + // decrease the size. + + account.realloc(0, false).unwrap(); + + assert_eq!(account.data_len(), 0); + assert_eq!(account.resize_delta(), -100); + + // Invalid reallocation. + + let invalid_realloc = account.realloc(10_000_000_001, false); + assert!(invalid_realloc.is_err()); + + // Reset to its original size. + + account.realloc(100, false).unwrap(); + + assert_eq!(account.data_len(), 100); + assert_eq!(account.resize_delta(), 0); + + // Consecutive reallocations. + + account.realloc(200, false).unwrap(); + account.realloc(50, false).unwrap(); + account.realloc(500, false).unwrap(); + + assert_eq!(account.data_len(), 500); + assert_eq!(account.resize_delta(), 400); + + let data = account.try_borrow_data().unwrap(); + assert_eq!(data.len(), 500); + } } From 15536c81cbb07d3cf625957bddcb8469d3097780 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Tue, 1 Jul 2025 12:31:08 +0100 Subject: [PATCH 27/51] Fix `assign` unsoundness (#180) * Fix assign unsoundness * Remove unsafe --- account-view/src/lib.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 27ad372b6..7f0faca4e 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -3,7 +3,7 @@ use core::{ marker::PhantomData, mem::ManuallyDrop, - ptr::NonNull, + ptr::{write, NonNull}, slice::{from_raw_parts, from_raw_parts_mut}, }; @@ -124,14 +124,9 @@ impl AccountInfo { } /// Program that owns this account. - /// - /// # Safety - /// - /// A reference returned by this method is invalidated when [`Self::assign`] - /// is called. #[inline(always)] - pub unsafe fn owner(&self) -> &Pubkey { - &(*self.raw).owner + pub fn owner(&self) -> &Pubkey { + unsafe { &(*self.raw).owner } } /// Indicates whether the transaction was signed by this account. @@ -187,18 +182,18 @@ impl AccountInfo { /// Checks if the account is owned by the given program. #[inline(always)] pub fn is_owned_by(&self, program: &Pubkey) -> bool { - unsafe { &(*self.raw).owner == program } + self.owner() == program } /// Changes the owner of the account. /// /// # Safety /// - /// Using this method invalidates any reference returned by [`Self::owner`]. + /// It is undefined behaviour to use this method while there is an active reference + /// to the `owner` returned by [`Self::owner`]. #[inline(always)] pub unsafe fn assign(&self, new_owner: &Pubkey) { - #[allow(invalid_reference_casting)] - core::ptr::write_volatile(&(*self.raw).owner as *const _ as *mut Pubkey, *new_owner); + write(&mut (*self.raw).owner, *new_owner); } /// Return true if the account borrow state is set to the given state. From 1d4ded3beb351e740931db4fc95395466a757d2f Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Wed, 16 Jul 2025 12:25:05 +0100 Subject: [PATCH 28/51] ci: Add spellcheck step (#164) * Add invoke instruction helper * Typos * Remove new helpers * Remove unused * Address review comments * Tweak inline attributes * Use invoke signed unchecked * Refactor inline * Renamed to with_bounds * Update docs * Revert change * Add constant length check * Add spellcheck step * Tweak action * Fix typos * More fixes * Yet more fixes * Fixes * Add j1 option * More and more fixes * Add missing acronym * Fix merge * Fix spelling * Fix spelling --- account-view/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 7f0faca4e..3066d9baf 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -48,8 +48,8 @@ pub(crate) struct Account { /// /// * lamport mutable borrow flag /// - `7 6 5 4 3 2 1 0` - /// - `x . . . . . . .`: `1` -> the lamport field can be mutably borrowed; - /// `0` -> there is an outstanding mutable borrow for the lamports. + /// - `x . . . . . . .`: `1` - the lamport field can be mutably borrowed; + /// `0` - there is an outstanding mutable borrow for the lamports. /// /// * lamport immutable borrow count /// - `7 6 5 4 3 2 1 0` @@ -59,8 +59,8 @@ pub(crate) struct Account { /// /// * data mutable borrow flag /// - `7 6 5 4 3 2 1 0` - /// - `. . . . x . . .`: `1` -> the account data can be mutably borrowed; - /// `0` -> there is an outstanding mutable borrow for the account data. + /// - `. . . . x . . .`: `1` - the account data can be mutably borrowed; + /// `0` - there is an outstanding mutable borrow for the account data. /// /// * data immutable borrow count /// - `7 6 5 4 3 2 1 0` @@ -189,7 +189,7 @@ impl AccountInfo { /// /// # Safety /// - /// It is undefined behaviour to use this method while there is an active reference + /// It is undefined behavior to use this method while there is an active reference /// to the `owner` returned by [`Self::owner`]. #[inline(always)] pub unsafe fn assign(&self, new_owner: &Pubkey) { From edad7823e2433059ed268900e8592ca525ee1b72 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Thu, 17 Jul 2025 00:16:29 +0100 Subject: [PATCH 29/51] Clarify the use of constant values (#200) * Add comments on constants * Improve offset comments * Add bitmask to dictionary * Renamed to field_at_offset --- account-view/src/lib.rs | 121 +++++++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 52 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 3066d9baf..3e18f5ece 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -22,12 +22,16 @@ pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; pub enum BorrowState { /// Mask to check whether an account is already borrowed. /// - /// This will test both data and lamports borrow state. + /// This will test both data and lamports borrow state. Any position + /// in the borrow byte that is not set means that the account + /// is borrowed in that state. Borrowed = 0b_1111_1111, /// Mask to check whether an account is already mutably borrowed. /// - /// This will test both data and lamports mutable borrow state. + /// This will test both data and lamports mutable borrow state. If + /// one of the mutably borrowed bits is not set, then the account + /// is mutably borrowed in that state. MutablyBorrowed = 0b_1000_1000, } @@ -261,18 +265,19 @@ impl AccountInfo { self.can_borrow_lamports()?; let borrow_state = self.raw as *mut u8; + // Use one immutable borrow for lamports by subtracting `1` from the + // lamports borrow counter bits; we are guaranteed that there is at + // least one immutable borrow available. + // // SAFETY: The `borrow_state` is a mutable pointer to the borrow state // of the account, which is guaranteed to be valid. - // - // "consumes" one immutable borrow for lamports; we are guaranteed - // that there is at least one immutable borrow available. - unsafe { *borrow_state -= 1 << LAMPORTS_SHIFT }; + unsafe { *borrow_state -= 1 << LAMPORTS_BORROW_SHIFT }; // return the reference to lamports Ok(Ref { value: unsafe { NonNull::from(&(*self.raw).lamports) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_shift: LAMPORTS_SHIFT, + borrow_shift: LAMPORTS_BORROW_SHIFT, marker: PhantomData, }) } @@ -284,18 +289,18 @@ impl AccountInfo { self.can_borrow_mut_lamports()?; let borrow_state = self.raw as *mut u8; + // Set the mutable lamports borrow bit to `0`; we are guaranteed + // that lamports are not already borrowed in any form. + // // SAFETY: The `borrow_state` is a mutable pointer to the borrow state // of the account, which is guaranteed to be valid. - // - // "consumes" the mutable borrow for lamports; we are guaranteed - // that lamports are not already borrowed in any form unsafe { *borrow_state &= 0b_0111_1111 }; // return the mutable reference to lamports Ok(RefMut { value: unsafe { NonNull::from(&mut (*self.raw).lamports) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_mask: LAMPORTS_MASK, + borrow_bitmask: LAMPORTS_MUTABLE_BORROW_BITMASK, marker: PhantomData, }) } @@ -314,12 +319,14 @@ impl AccountInfo { pub fn can_borrow_lamports(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; - // check if mutable borrow is already taken - if borrow_state & 0b_1000_0000 != 0b_1000_0000 { + // Check whether the mutable lamports borrow bit is already in + // use (value `0`) or not. If it is `0`, then the borrow will fail. + if borrow_state & LAMPORTS_MUTABLE_BORROW_BITMASK == 0 { return Err(ProgramError::AccountBorrowFailed); } - // check if we have reached the max immutable borrow count + // Check whether we have reached the maximum immutable lamports borrow count + // or not, i.e., it fails when all immutable lamports borrow bits are `0`. if borrow_state & 0b_0111_0000 == 0 { return Err(ProgramError::AccountBorrowFailed); } @@ -341,7 +348,8 @@ impl AccountInfo { pub fn can_borrow_mut_lamports(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; - // check if any borrow (mutable or immutable) is already taken for lamports + // Check whether any (mutable or immutable) lamports borrow bits are + // in use (value `0`) or not. if borrow_state & 0b_1111_0000 != 0b_1111_0000 { return Err(ProgramError::AccountBorrowFailed); } @@ -356,18 +364,19 @@ impl AccountInfo { self.can_borrow_data()?; let borrow_state = self.raw as *mut u8; + // Use one immutable borrow for data by subtracting `1` from the data + // borrow counter bits; we are guaranteed that there is at least one + // immutable borrow available. + // // SAFETY: The `borrow_state` is a mutable pointer to the borrow state // of the account, which is guaranteed to be valid. - // - // "consumes" one immutable borrow for data; we are guaranteed - // that there is at least one immutable borrow available unsafe { *borrow_state -= 1 }; // return the reference to data Ok(Ref { value: unsafe { NonNull::from(from_raw_parts(self.data_ptr(), self.data_len())) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_shift: DATA_SHIFT, + borrow_shift: DATA_BORROW_SHIFT, marker: PhantomData, }) } @@ -379,18 +388,18 @@ impl AccountInfo { self.can_borrow_mut_data()?; let borrow_state = self.raw as *mut u8; + // Set the mutable data borrow bit to `0`; we are guaranteed that account + // data is not already borrowed in any form. + // // SAFETY: The `borrow_state` is a mutable pointer to the borrow state // of the account, which is guaranteed to be valid. - // - // "consumes" the mutable borrow for account data; we are guaranteed - // that account data are not already borrowed in any form unsafe { *borrow_state &= 0b_1111_0111 }; // return the mutable reference to data Ok(RefMut { value: unsafe { NonNull::from(from_raw_parts_mut(self.data_ptr(), self.data_len())) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_mask: DATA_MASK, + borrow_bitmask: DATA_MUTABLE_BORROW_BITMASK, marker: PhantomData, }) } @@ -409,13 +418,14 @@ impl AccountInfo { pub fn can_borrow_data(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; - // check if mutable data borrow is already taken (most significant bit - // of the data borrow state) - if borrow_state & 0b_0000_1000 == 0 { + // Check whether the mutable data borrow bit is already in + // use (value `0`) or not. If it is `0`, then the borrow will fail. + if borrow_state & DATA_MUTABLE_BORROW_BITMASK == 0 { return Err(ProgramError::AccountBorrowFailed); } - // check if we have reached the max immutable data borrow count (7) + // Check whether we have reached the maximum immutable data borrow count + // or not, i.e., it fails when all immutable data borrow bits are `0`. if borrow_state & 0b_0000_0111 == 0 { return Err(ProgramError::AccountBorrowFailed); } @@ -437,7 +447,8 @@ impl AccountInfo { pub fn can_borrow_mut_data(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; - // check if any borrow (mutable or immutable) is already taken for data + // Check whether any (mutable or immutable) data borrow bits are + // in use (value `0`) or not. if borrow_state & 0b_0000_1111 != 0b_0000_1111 { return Err(ProgramError::AccountBorrowFailed); } @@ -610,11 +621,17 @@ impl AccountInfo { } } -/// Bytes to shift to get to the borrow state of lamports. -const LAMPORTS_SHIFT: u8 = 4; +/// Number of bits of the [`Account::borrow_state`] flag to shift to get to +/// the borrow state bits for lamports. +/// - `7 6 5 4 3 2 1 0` +/// - `x x x x . . . .` +const LAMPORTS_BORROW_SHIFT: u8 = 4; -/// Bytes to shift to get to the borrow state of data. -const DATA_SHIFT: u8 = 0; +/// Number of bits of the [`Account::borrow_state`] flag to shift to get to +/// the borrow state bits for account data. +/// - `7 6 5 4 3 2 1 0` +/// - `. . . . x x x x` +const DATA_BORROW_SHIFT: u8 = 0; /// Reference to account data or lamports with checked borrow rules. pub struct Ref<'a, T: ?Sized> { @@ -682,18 +699,18 @@ impl Drop for Ref<'_, T> { } /// Mask representing the mutable borrow flag for lamports. -const LAMPORTS_MASK: u8 = 0b_1000_0000; +const LAMPORTS_MUTABLE_BORROW_BITMASK: u8 = 0b_1000_0000; /// Mask representing the mutable borrow flag for data. -const DATA_MASK: u8 = 0b_0000_1000; +const DATA_MUTABLE_BORROW_BITMASK: u8 = 0b_0000_1000; /// Mutable reference to account data or lamports with checked borrow rules. pub struct RefMut<'a, T: ?Sized> { value: NonNull, state: NonNull, - /// Indicates the type of borrow (lamports or data) by representing the - /// mutable borrow mask. - borrow_mask: u8, + /// Indicates borrowed field (lamports or data) by storing the bitmask + /// representing the mutable borrow. + borrow_bitmask: u8, /// The `value` raw pointer is only valid while the `&'a T` lives so we claim /// to hold a reference to it. marker: PhantomData<&'a mut T>, @@ -712,7 +729,7 @@ impl<'a, T: ?Sized> RefMut<'a, T> { RefMut { value: NonNull::from(f(&mut *orig)), state: orig.state, - borrow_mask: orig.borrow_mask, + borrow_bitmask: orig.borrow_bitmask, marker: PhantomData, } } @@ -732,7 +749,7 @@ impl<'a, T: ?Sized> RefMut<'a, T> { Ok(RefMut { value, state: orig.state, - borrow_mask: orig.borrow_mask, + borrow_bitmask: orig.borrow_bitmask, marker: PhantomData, }) } @@ -756,7 +773,7 @@ impl core::ops::DerefMut for RefMut<'_, T> { impl Drop for RefMut<'_, T> { fn drop(&mut self) { // unset the mutable borrow flag - unsafe { *self.state.as_mut() |= self.borrow_mask }; + unsafe { *self.state.as_mut() |= self.borrow_bitmask }; } } @@ -771,11 +788,11 @@ mod tests { #[test] fn test_data_ref() { let data: [u8; 4] = [0, 1, 2, 3]; - let mut state = NOT_BORROWED - (1 << DATA_SHIFT); + let mut state = NOT_BORROWED - (1 << DATA_BORROW_SHIFT); let ref_data = Ref { value: NonNull::from(&data), - borrow_shift: DATA_SHIFT, + borrow_shift: DATA_BORROW_SHIFT, // borrow state must be a mutable reference state: NonNull::from(&mut state), marker: PhantomData, @@ -783,19 +800,19 @@ mod tests { let new_ref = Ref::map(ref_data, |data| &data[1]); - assert_eq!(state, NOT_BORROWED - (1 << DATA_SHIFT)); + assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); assert_eq!(*new_ref, 1); let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&3)) else { unreachable!() }; - assert_eq!(state, NOT_BORROWED - (1 << DATA_SHIFT)); + assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); assert_eq!(*new_ref, 3); let new_ref = Ref::filter_map(new_ref, |_| Option::<&u8>::None); - assert_eq!(state, NOT_BORROWED - (1 << DATA_SHIFT)); + assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); assert!(new_ref.is_err()); drop(new_ref); @@ -806,11 +823,11 @@ mod tests { #[test] fn test_lamports_ref() { let lamports: u64 = 10000; - let mut state = NOT_BORROWED - (1 << LAMPORTS_SHIFT); + let mut state = NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT); let ref_lamports = Ref { value: NonNull::from(&lamports), - borrow_shift: LAMPORTS_SHIFT, + borrow_shift: LAMPORTS_BORROW_SHIFT, // borrow state must be a mutable reference state: NonNull::from(&mut state), marker: PhantomData, @@ -818,19 +835,19 @@ mod tests { let new_ref = Ref::map(ref_lamports, |_| &1000); - assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_SHIFT)); + assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT)); assert_eq!(*new_ref, 1000); let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&2000)) else { unreachable!() }; - assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_SHIFT)); + assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT)); assert_eq!(*new_ref, 2000); let new_ref = Ref::filter_map(new_ref, |_| Option::<&i32>::None); - assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_SHIFT)); + assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT)); assert!(new_ref.is_err()); drop(new_ref); @@ -845,7 +862,7 @@ mod tests { let ref_data = RefMut { value: NonNull::from(&mut data), - borrow_mask: DATA_MASK, + borrow_bitmask: DATA_MUTABLE_BORROW_BITMASK, // borrow state must be a mutable reference state: NonNull::from(&mut state), marker: PhantomData, @@ -873,7 +890,7 @@ mod tests { let ref_lamports = RefMut { value: NonNull::from(&mut lamports), - borrow_mask: LAMPORTS_MASK, + borrow_bitmask: LAMPORTS_MUTABLE_BORROW_BITMASK, // borrow state must be a mutable reference state: NonNull::from(&mut state), marker: PhantomData, From 36ec9b81b8bbe2b53852bb52eaad1d6d1de8c259 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Thu, 17 Jul 2025 20:24:25 +0100 Subject: [PATCH 30/51] Ignore `zero_init` parameter (#203) Ignore zero_init parameter --- account-view/src/lib.rs | 55 ++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 3e18f5ece..4eebc7184 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -456,32 +456,42 @@ impl AccountInfo { Ok(()) } - /// Realloc the account's data and optionally zero-initialize the new - /// memory. + /// Realloc (either truncating or zero extending) the account's data. /// /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes /// within an instruction. /// /// # Important /// - /// Memory used to grow is already zero-initialized upon program - /// entrypoint and re-zeroing it wastes compute units. If within the same - /// call a program reallocs from larger to smaller and back to larger again - /// the new space could contain stale data. Pass `true` for `zero_init` in - /// this case, otherwise compute units will be wasted re-zero-initializing. + /// The use of the `zero_init` parameter, which indicated whether the newly + /// allocated memory should be zero-initialized or not, is now deprecated and + /// ignored. The method will always zero-initialize the newly allocated memory + /// if the new length is larger than the current data length. This is the same + /// behavior as [`Self::resize`]. /// - /// Note that `zero_init` being `false` will not be supported by the - /// program runtime in the future. Programs should not rely on being able to - /// access stale data and use [`Self::resize`] instead. + /// This method makes assumptions about the layout and location of memory + /// referenced by `AccountInfo` fields. It should only be called for + /// instances of `AccountInfo` that were created by the runtime and received + /// in the `process_instruction` entrypoint of a program. + #[deprecated(since = "0.9.0", note = "Use AccountInfo::resize() instead")] + #[inline(always)] + pub fn realloc(&self, new_len: usize, _zero_init: bool) -> Result<(), ProgramError> { + self.resize(new_len) + } + + /// Resize (either truncating or zero extending) the account's data. /// - /// # Safety + /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes + /// within an instruction. + /// + /// # Important /// /// This method makes assumptions about the layout and location of memory /// referenced by `AccountInfo` fields. It should only be called for /// instances of `AccountInfo` that were created by the runtime and received /// in the `process_instruction` entrypoint of a program. - #[deprecated(since = "0.9.0", note = "Use AccountInfo::resize() instead")] - pub fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), ProgramError> { + #[inline] + pub fn resize(&self, new_len: usize) -> Result<(), ProgramError> { // Check wheather the account data is already borrowed. self.can_borrow_mut_data()?; @@ -509,7 +519,7 @@ impl AccountInfo { (*self.raw).resize_delta = accumulated_resize_delta; } - if zero_init && difference > 0 { + if difference > 0 { unsafe { #[cfg(target_os = "solana")] sol_memset_( @@ -529,23 +539,6 @@ impl AccountInfo { Ok(()) } - /// Resize (either truncating or zero extending) the account's data. - /// - /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes - /// within an instruction. - /// - /// # Important - /// - /// This method makes assumptions about the layout and location of memory - /// referenced by `AccountInfo` fields. It should only be called for - /// instances of `AccountInfo` that were created by the runtime and received - /// in the `process_instruction` entrypoint of a program. - #[inline] - pub fn resize(&self, new_len: usize) -> Result<(), ProgramError> { - #[allow(deprecated)] - self.realloc(new_len, true) - } - /// Zero out the the account's data length, lamports and owner fields, effectively /// closing the account. /// From 336a47855e46b38664f7f8e1a2ab937621994f67 Mon Sep 17 00:00:00 2001 From: Sammy Harris <41593264+stegaBOB@users.noreply.github.com> Date: Mon, 25 Aug 2025 19:45:26 -0500 Subject: [PATCH 31/51] Add resize_unchecked method to account info (#230) * add resize_unchecked method to account info * Apply suggestion from @febo Co-authored-by: Fernando Otero * Apply suggestion from @febo Co-authored-by: Fernando Otero * Apply suggestion from @febo Co-authored-by: Fernando Otero --------- Co-authored-by: Fernando Otero --- account-view/src/lib.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 4eebc7184..c1727918a 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -495,6 +495,22 @@ impl AccountInfo { // Check wheather the account data is already borrowed. self.can_borrow_mut_data()?; + // SAFETY: + // We are checking if the account data is already borrowed, so we are safe to call + unsafe { self.resize_unchecked(new_len) } + } + + /// Resize (either truncating or zero extending) the account's data. + /// + /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes + /// + /// # Safety + /// + /// This method is unsafe because it does not check if the account data is already + /// borrowed. The caller must guarantee that there are no active borrows to the account + /// data. + #[inline(always)] + pub unsafe fn resize_unchecked(&self, new_len: usize) -> Result<(), ProgramError> { // Account length is always `< i32::MAX`... let current_len = self.data_len() as i32; // ...so the new length must fit in an `i32`. From b50eddd489e6c18a09edca0d33b9227471e256b7 Mon Sep 17 00:00:00 2001 From: Sammy Harris <41593264+stegaBOB@users.noreply.github.com> Date: Tue, 26 Aug 2025 11:35:43 -0500 Subject: [PATCH 32/51] Feat: Add debug/copy derives and enable missing debug/copy lint (#228) * Add debug/copy derives and enable missing debug/copy lint * Update sdk/pinocchio/src/sysvars/rent.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/entrypoint/mod.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/instruction.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/sysvars/clock.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/sysvars/fees.rs * Update sdk/pinocchio/src/sysvars/fees.rs * Update sdk/pinocchio/src/sysvars/instructions.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/sysvars/instructions.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/sysvars/instructions.rs Co-authored-by: Fernando Otero * Update sdk/pinocchio/src/sysvars/clock.rs * Fix syntax error in Instructions struct derive macro --------- Co-authored-by: Fernando Otero --- account-view/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index c1727918a..2a3c67486 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -18,7 +18,7 @@ pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; /// Represents masks for borrow state of an account. #[repr(u8)] -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub enum BorrowState { /// Mask to check whether an account is already borrowed. /// @@ -112,7 +112,7 @@ pub(crate) struct Account { /// used to track borrows of the account data and lamports, given that an /// account can be "shared" across multiple `AccountInfo` instances. #[repr(C)] -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct AccountInfo { /// Raw (pointer to) account data. /// @@ -643,6 +643,7 @@ const LAMPORTS_BORROW_SHIFT: u8 = 4; const DATA_BORROW_SHIFT: u8 = 0; /// Reference to account data or lamports with checked borrow rules. +#[derive(Debug)] pub struct Ref<'a, T: ?Sized> { value: NonNull, state: NonNull, @@ -714,6 +715,7 @@ const LAMPORTS_MUTABLE_BORROW_BITMASK: u8 = 0b_1000_0000; const DATA_MUTABLE_BORROW_BITMASK: u8 = 0b_0000_1000; /// Mutable reference to account data or lamports with checked borrow rules. +#[derive(Debug)] pub struct RefMut<'a, T: ?Sized> { value: NonNull, state: NonNull, From 00e3e754e231bfd11ea0145553c09eb53f6ea728 Mon Sep 17 00:00:00 2001 From: Sammy Harris <41593264+stegaBOB@users.noreply.github.com> Date: Wed, 27 Aug 2025 05:56:08 -0500 Subject: [PATCH 33/51] Feat: make AccountInfo::data_ptr public (#232) * make data_ptr public * Update sdk/pinocchio/src/account_info.rs Co-authored-by: Fernando Otero * add some tests for data ptr * Fix spelling --------- Co-authored-by: Fernando Otero --- account-view/src/lib.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 2a3c67486..8da78c203 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -625,7 +625,13 @@ impl AccountInfo { } /// Returns the memory address of the account data. - fn data_ptr(&self) -> *mut u8 { + /// # Important + /// + /// Obtaining the raw pointer itself is safe, but de-referencing it requires + /// the caller to uphold Rust's aliasing rules. It is undefined behavior to de-reference + /// the pointer or write through it while any safe reference (e.g., from any of `borrow_data` + /// or `borrow_mut_data` methods) to the same data is still alive. + pub fn data_ptr(&self) -> *mut u8 { unsafe { (self.raw as *mut u8).add(core::mem::size_of::()) } } } @@ -924,8 +930,8 @@ mod tests { #[test] fn test_borrow_data() { // 8-bytes aligned account data. - let mut data = [0u64; size_of::() / size_of::()]; - // Set the borrow state. + let mut data = [0u64; size_of::() / size_of::() + 1]; // extra byte at end for account data + // Set the borrow state. data[0] = NOT_BORROWED as u64; let account_info = AccountInfo { raw: data.as_mut_ptr() as *mut Account, @@ -937,6 +943,13 @@ mod tests { assert!(account_info.can_borrow_lamports().is_ok()); assert!(account_info.can_borrow_mut_lamports().is_ok()); + // It should be sound to mutate the data through the data pointer while no other borrows exist + let data_ptr = account_info.data_ptr(); + unsafe { + let data = core::slice::from_raw_parts_mut(data_ptr, 1); // Data is 1 byte long! + data[0] = 1; + } + // Borrow immutable data (7 immutable borrows available). const ACCOUNT_REF: MaybeUninit> = MaybeUninit::>::uninit(); let mut refs = [ACCOUNT_REF; 7]; @@ -969,6 +982,8 @@ mod tests { // Borrow mutable data. let ref_mut = account_info.try_borrow_mut_data().unwrap(); + // It should be sound to get the data pointer while the data is borrowed as long as we don't use it + let _data_ptr = account_info.data_ptr(); // Check that we cannot borrow the data anymore. assert!(account_info.can_borrow_data().is_err()); @@ -1070,10 +1085,17 @@ mod tests { assert_eq!(account.data_len(), 100); assert_eq!(account.resize_delta(), 0); + // We should be able to get the data pointer whenever as long as we don't use it while the data is borrowed + let data_ptr_before = account.data_ptr(); + // increase the size. account.realloc(200, false).unwrap(); + let data_ptr_after = account.data_ptr(); + // The data pointer should point to the same address regardless of the reallocation + assert_eq!(data_ptr_before, data_ptr_after); + assert_eq!(account.data_len(), 200); assert_eq!(account.resize_delta(), 100); From 109ce33521502dedb0561e6c8b8a5b467569b275 Mon Sep 17 00:00:00 2001 From: Sammy Harris <41593264+stegaBOB@users.noreply.github.com> Date: Wed, 27 Aug 2025 08:31:47 -0500 Subject: [PATCH 34/51] Feat: Add try_maps on AccountInfo Ref/RefMut (#229) * Add try_maps on AccountInfo Ref/RefMut * update tests and api for try map ref --- account-view/src/lib.rs | 73 ++++++++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 8da78c203..ccdb0e453 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -670,7 +670,6 @@ impl<'a, T: ?Sized> Ref<'a, T> { { // Avoid decrementing the borrow flag on Drop. let orig = ManuallyDrop::new(orig); - Ref { value: NonNull::from(f(&*orig)), state: orig.state, @@ -679,6 +678,27 @@ impl<'a, T: ?Sized> Ref<'a, T> { } } + /// Tries to makes a new `Ref` for a component of the borrowed data. + /// On failure, the original guard is returned alongside with the error + /// returned by the closure. + #[inline] + pub fn try_map( + orig: Ref<'a, T>, + f: impl FnOnce(&T) -> Result<&U, E>, + ) -> Result, (Self, E)> { + // Avoid decrementing the borrow flag on Drop. + let orig = ManuallyDrop::new(orig); + match f(&*orig) { + Ok(value) => Ok(Ref { + value: NonNull::from(value), + state: orig.state, + borrow_shift: orig.borrow_shift, + marker: PhantomData, + }), + Err(e) => Err((ManuallyDrop::into_inner(orig), e)), + } + } + /// Filters and maps a reference to a new type. #[inline] pub fn filter_map(orig: Ref<'a, T>, f: F) -> Result, Self> @@ -742,7 +762,6 @@ impl<'a, T: ?Sized> RefMut<'a, T> { { // Avoid decrementing the borrow flag on Drop. let mut orig = ManuallyDrop::new(orig); - RefMut { value: NonNull::from(f(&mut *orig)), state: orig.state, @@ -751,6 +770,27 @@ impl<'a, T: ?Sized> RefMut<'a, T> { } } + /// Tries to makes a new `RefMut` for a component of the borrowed data. + /// On failure, the original guard is returned alongside with the error + /// returned by the closure. + #[inline] + pub fn try_map( + orig: RefMut<'a, T>, + f: impl FnOnce(&mut T) -> Result<&mut U, E>, + ) -> Result, (Self, E)> { + // Avoid decrementing the borrow flag on Drop. + let mut orig = ManuallyDrop::new(orig); + match f(&mut *orig) { + Ok(value) => Ok(RefMut { + value: NonNull::from(value), + state: orig.state, + borrow_bitmask: orig.borrow_bitmask, + marker: PhantomData, + }), + Err(e) => Err((ManuallyDrop::into_inner(orig), e)), + } + } + /// Filters and maps a mutable reference to a new type. #[inline] pub fn filter_map(orig: RefMut<'a, T>, f: F) -> Result, Self> @@ -759,17 +799,13 @@ impl<'a, T: ?Sized> RefMut<'a, T> { { // Avoid decrementing the mutable borrow flag on Drop. let mut orig = ManuallyDrop::new(orig); - match f(&mut *orig) { - Some(value) => { - let value = NonNull::from(value); - Ok(RefMut { - value, - state: orig.state, - borrow_bitmask: orig.borrow_bitmask, - marker: PhantomData, - }) - } + Some(value) => Ok(RefMut { + value: NonNull::from(value), + state: orig.state, + borrow_bitmask: orig.borrow_bitmask, + marker: PhantomData, + }), None => Err(ManuallyDrop::into_inner(orig)), } } @@ -827,6 +863,19 @@ mod tests { assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); assert_eq!(*new_ref, 3); + let Ok(new_ref) = Ref::try_map::<_, u8>(new_ref, |_| Ok(&4)) else { + unreachable!() + }; + + assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); + assert_eq!(*new_ref, 4); + + let (new_ref, err) = Ref::try_map::(new_ref, |_| Err(5)).unwrap_err(); + assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); + assert_eq!(err, 5); + // Unchanged + assert_eq!(*new_ref, 4); + let new_ref = Ref::filter_map(new_ref, |_| Option::<&u8>::None); assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); From 7af3d31fc22d9d851c44a080465bfb5a72d225a8 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Mon, 8 Sep 2025 09:09:45 +0100 Subject: [PATCH 35/51] pinocchio: Move `NON_DUP_MARKER` const (#245) Move NON_DUP_MARKER const --- account-view/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index ccdb0e453..88483ed68 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -834,7 +834,7 @@ impl Drop for RefMut<'_, T> { mod tests { use core::mem::{size_of, MaybeUninit}; - use crate::NON_DUP_MARKER as NOT_BORROWED; + use crate::entrypoint::NON_DUP_MARKER as NOT_BORROWED; use super::*; From b6e20e7880aa2ea5f4f4e2746a0619d3a12cd6b4 Mon Sep 17 00:00:00 2001 From: vetclippy Date: Thu, 11 Sep 2025 00:27:59 +0800 Subject: [PATCH 36/51] chore: fix typo in comment (#240) Signed-off-by: vetclippy --- account-view/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 88483ed68..9422be792 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -492,7 +492,7 @@ impl AccountInfo { /// in the `process_instruction` entrypoint of a program. #[inline] pub fn resize(&self, new_len: usize) -> Result<(), ProgramError> { - // Check wheather the account data is already borrowed. + // Check whether the account data is already borrowed. self.can_borrow_mut_data()?; // SAFETY: From dd6266b271fdf5f458b9afe7f2d7f5a3337bf7d0 Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 12 Sep 2025 00:18:11 +0100 Subject: [PATCH 37/51] pinocchio: Add `pubkey_eq` helper (#248) * Add pubkey_eq helper * Fix typo * Update pubkey comparison * Add proptest * Add unlikely * Replace proptest --- account-view/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 9422be792..8ba2499e3 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -10,7 +10,11 @@ use core::{ #[cfg(target_os = "solana")] use crate::syscalls::sol_memset_; -use crate::{program_error::ProgramError, pubkey::Pubkey, ProgramResult}; +use crate::{ + program_error::ProgramError, + pubkey::{pubkey_eq, Pubkey}, + ProgramResult, +}; /// Maximum number of bytes a program may add to an account during a /// single top-level instruction. @@ -186,7 +190,7 @@ impl AccountInfo { /// Checks if the account is owned by the given program. #[inline(always)] pub fn is_owned_by(&self, program: &Pubkey) -> bool { - self.owner() == program + pubkey_eq(self.owner(), program) } /// Changes the owner of the account. From 2a14665f87354134385e251e4c58ae6325286e1b Mon Sep 17 00:00:00 2001 From: Fernando Otero Date: Fri, 19 Sep 2025 21:04:31 +0100 Subject: [PATCH 38/51] pinocchio: Add `AccountInfo` invariant details (#254) Add invariant details --- account-view/src/lib.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 8ba2499e3..fdb331bf0 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -41,8 +41,10 @@ pub enum BorrowState { /// Raw account data. /// -/// This data is wrapped in an `AccountInfo` struct, which provides safe access -/// to the data. +/// This struct is wrapped by [`AccountInfo`], which provides safe access +/// to account information. At runtime, the account's data is serialized +/// directly after the `Account` struct in memory, with its size specified +/// by [`Account::data_len`]. #[repr(C)] #[derive(Clone, Copy, Default)] pub(crate) struct Account { @@ -112,9 +114,20 @@ pub(crate) struct Account { /// Wrapper struct for an `Account`. /// -/// This struct provides safe access to the data in an `Account`. It is also -/// used to track borrows of the account data and lamports, given that an -/// account can be "shared" across multiple `AccountInfo` instances. +/// This struct provides safe access to the data in an `Account`. It is +/// also used to track borrows of the account data and lamports, given +/// that an account can be "shared" across multiple `AccountInfo` +/// instances. +/// +/// # Invariants +/// +/// - The `raw` pointer must be valid and point to memory containing an +/// `Account` struct, immediately followed by the account's data region. +/// - The length of the account data must exactly match the value stored in +/// `Account::data_len`. +/// +/// These conditions must always hold for any `AccountInfo` created from +/// a raw pointer. #[repr(C)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct AccountInfo { From 61e1c1eb284984b10d0a4f9a107331403dbda34e Mon Sep 17 00:00:00 2001 From: febo Date: Mon, 22 Sep 2025 11:35:48 +0100 Subject: [PATCH 39/51] Add account view --- Cargo.lock | 9 ++ Cargo.toml | 2 + account-view/Cargo.toml | 21 ++++ account-view/src/lib.rs | 242 +++++++++++++++++----------------------- 4 files changed, 132 insertions(+), 142 deletions(-) create mode 100644 account-view/Cargo.toml diff --git a/Cargo.lock b/Cargo.lock index 4892921bc..0f426c77c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2756,6 +2756,15 @@ dependencies = [ "solana-program-memory", ] +[[package]] +name = "solana-account-view" +version = "0.0.0" +dependencies = [ + "solana-address", + "solana-program-entrypoint", + "solana-program-error", +] + [[package]] name = "solana-address" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index 6937d0e58..937d55757 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ members = [ "account", "account-info", + "account-view", "address", "address-lookup-table-interface", "atomic-u64", @@ -218,6 +219,7 @@ signal-hook = "0.3.17" siphasher = "0.3.11" solana-account = { path = "account", version = "3.0.0" } solana-account-info = { path = "account-info", version = "3.0.0" } +solana-account-view = { path = "account-view", version = "0.0.0" } solana-address = { path = "address", version = "1.0.0" } solana-address-lookup-table-interface = { path = "address-lookup-table-interface", version = "3.0.0" } solana-atomic-u64 = { path = "atomic-u64", version = "3.0.0" } diff --git a/account-view/Cargo.toml b/account-view/Cargo.toml new file mode 100644 index 000000000..0088a2316 --- /dev/null +++ b/account-view/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "solana-account-view" +description = "A lightweight representation of a runtime account" +documentation = "https://docs.rs/solana-account-view" +version = "0.0.0" +rust-version = "1.81.0" +authors = { workspace = true } +repository = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +edition = { workspace = true } + +[dependencies] +solana-address = { workspace = true } +solana-program-error = { workspace = true } + +[dev-dependencies] +solana-program-entrypoint = { workspace = true } + +[lints] +workspace = true diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index fdb331bf0..05f332572 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,19 +1,16 @@ //! Data structures to represent account information. -use core::{ - marker::PhantomData, - mem::ManuallyDrop, - ptr::{write, NonNull}, - slice::{from_raw_parts, from_raw_parts_mut}, -}; - -#[cfg(target_os = "solana")] -use crate::syscalls::sol_memset_; - -use crate::{ - program_error::ProgramError, - pubkey::{pubkey_eq, Pubkey}, - ProgramResult, +#![no_std] + +use { + core::{ + marker::PhantomData, + mem::ManuallyDrop, + ptr::{write, write_bytes, NonNull}, + slice::{from_raw_parts, from_raw_parts_mut}, + }, + solana_address::Address, + solana_program_error::{ProgramError, ProgramResult}, }; /// Maximum number of bytes a program may add to an account during a @@ -41,13 +38,13 @@ pub enum BorrowState { /// Raw account data. /// -/// This struct is wrapped by [`AccountInfo`], which provides safe access +/// This struct is wrapped by [`AccountView`], which provides safe access /// to account information. At runtime, the account's data is serialized /// directly after the `Account` struct in memory, with its size specified /// by [`Account::data_len`]. #[repr(C)] #[derive(Clone, Copy, Default)] -pub(crate) struct Account { +pub struct Account { /// Borrow state for lamports and account data. /// /// This reuses the memory reserved for the duplicate flag in the @@ -80,16 +77,16 @@ pub(crate) struct Account { /// Note that this values are shared across `AccountInfo`s over the /// same account, e.g., in case of duplicated accounts, they share /// the same borrow state. - pub(crate) borrow_state: u8, + pub borrow_state: u8, /// Indicates whether the transaction was signed by this account. - is_signer: u8, + pub is_signer: u8, /// Indicates whether the account is writable. - is_writable: u8, + pub is_writable: u8, /// Indicates whether this account represents a program. - executable: u8, + pub executable: u8, /// Difference between the original data length and the current /// data length. @@ -97,26 +94,26 @@ pub(crate) struct Account { /// This is used to track the original data length of the account /// when the account is resized. The runtime guarantees that this /// value is zero at the start of the instruction. - resize_delta: i32, + pub resize_delta: i32, /// Public key of the account. - key: Pubkey, + pub key: Address, /// Program that owns this account. Modifiable by programs. - owner: Pubkey, + pub owner: Address, /// The lamports in the account. Modifiable by programs. - lamports: u64, + pub lamports: u64, /// Length of the data. Modifiable by programs. - pub(crate) data_len: u64, + pub data_len: u64, } /// Wrapper struct for an `Account`. /// /// This struct provides safe access to the data in an `Account`. It is /// also used to track borrows of the account data and lamports, given -/// that an account can be "shared" across multiple `AccountInfo` +/// that an account can be "shared" across multiple `AccountView` /// instances. /// /// # Invariants @@ -126,27 +123,27 @@ pub(crate) struct Account { /// - The length of the account data must exactly match the value stored in /// `Account::data_len`. /// -/// These conditions must always hold for any `AccountInfo` created from +/// These conditions must always hold for any `AccountView` created from /// a raw pointer. #[repr(C)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub struct AccountInfo { +pub struct AccountView { /// Raw (pointer to) account data. /// - /// Note that this is a pointer can be shared across multiple `AccountInfo`. - pub(crate) raw: *mut Account, + /// Note that this is a pointer can be shared across multiple `AccountView`. + raw: *mut Account, } -impl AccountInfo { +impl AccountView { /// Public key of the account. #[inline(always)] - pub fn key(&self) -> &Pubkey { + pub fn key(&self) -> &Address { unsafe { &(*self.raw).key } } /// Program that owns this account. #[inline(always)] - pub fn owner(&self) -> &Pubkey { + pub fn owner(&self) -> &Address { unsafe { &(*self.raw).owner } } @@ -196,14 +193,14 @@ impl AccountInfo { /// /// An account is considered empty if the data length is zero. #[inline(always)] - pub fn data_is_empty(&self) -> bool { + pub fn is_data_empty(&self) -> bool { self.data_len() == 0 } /// Checks if the account is owned by the given program. #[inline(always)] - pub fn is_owned_by(&self, program: &Pubkey) -> bool { - pubkey_eq(self.owner(), program) + pub fn owned_by(&self, program: &Address) -> bool { + self.owner() == program } /// Changes the owner of the account. @@ -213,7 +210,7 @@ impl AccountInfo { /// It is undefined behavior to use this method while there is an active reference /// to the `owner` returned by [`Self::owner`]. #[inline(always)] - pub unsafe fn assign(&self, new_owner: &Pubkey) { + pub unsafe fn assign(&self, new_owner: &Address) { write(&mut (*self.raw).owner, *new_owner); } @@ -248,7 +245,7 @@ impl AccountInfo { /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[allow(clippy::mut_from_ref)] #[inline(always)] - pub unsafe fn borrow_mut_lamports_unchecked(&self) -> &mut u64 { + pub unsafe fn borrow_lamports_unchecked_mut(&self) -> &mut u64 { &mut (*self.raw).lamports } @@ -260,7 +257,7 @@ impl AccountInfo { /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[inline(always)] pub unsafe fn borrow_data_unchecked(&self) -> &[u8] { - core::slice::from_raw_parts(self.data_ptr(), self.data_len()) + from_raw_parts(self.data_ptr(), self.data_len()) } /// Returns a mutable reference to the data in the account. @@ -271,8 +268,8 @@ impl AccountInfo { /// flag untouched. Useful when an instruction has verified non-duplicate accounts. #[allow(clippy::mut_from_ref)] #[inline(always)] - pub unsafe fn borrow_mut_data_unchecked(&self) -> &mut [u8] { - core::slice::from_raw_parts_mut(self.data_ptr(), self.data_len()) + pub unsafe fn borrow_data_unchecked_mut(&self) -> &mut [u8] { + from_raw_parts_mut(self.data_ptr(), self.data_len()) } /// Tries to get a read-only reference to the lamport field, failing if the @@ -301,9 +298,9 @@ impl AccountInfo { /// Tries to get a read only reference to the lamport field, failing if the field /// is already borrowed in any form. - pub fn try_borrow_mut_lamports(&self) -> Result, ProgramError> { + pub fn try_borrow_lamports_mut(&self) -> Result, ProgramError> { // check if the account lamports are already borrowed - self.can_borrow_mut_lamports()?; + self.can_borrow_lamports_mut()?; let borrow_state = self.raw as *mut u8; // Set the mutable lamports borrow bit to `0`; we are guaranteed @@ -322,14 +319,6 @@ impl AccountInfo { }) } - /// Checks if it is possible to get a read-only reference to the lamport field, - /// failing if the field is already mutable borrowed or if 7 borrows already exist. - #[deprecated(since = "0.8.4", note = "Use `can_borrow_lamports` instead")] - #[inline(always)] - pub fn check_borrow_lamports(&self) -> Result<(), ProgramError> { - self.can_borrow_lamports() - } - /// Checks if it is possible to get a read-only reference to the lamport field, /// failing if the field is already mutable borrowed or if `7` borrows already exist. #[inline(always)] @@ -353,16 +342,8 @@ impl AccountInfo { /// Checks if it is possible to get a mutable reference to the lamport field, /// failing if the field is already borrowed in any form. - #[deprecated(since = "0.8.4", note = "Use `can_borrow_mut_lamports` instead")] #[inline(always)] - pub fn check_borrow_mut_lamports(&self) -> Result<(), ProgramError> { - self.can_borrow_mut_lamports() - } - - /// Checks if it is possible to get a mutable reference to the lamport field, - /// failing if the field is already borrowed in any form. - #[inline(always)] - pub fn can_borrow_mut_lamports(&self) -> Result<(), ProgramError> { + pub fn can_borrow_lamports_mut(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // Check whether any (mutable or immutable) lamports borrow bits are @@ -400,9 +381,9 @@ impl AccountInfo { /// Tries to get a mutable reference to the data field, failing if the field /// is already borrowed in any form. - pub fn try_borrow_mut_data(&self) -> Result, ProgramError> { + pub fn try_borrow_data_mut(&self) -> Result, ProgramError> { // check if the account data is already borrowed - self.can_borrow_mut_data()?; + self.can_borrow_data_mut()?; let borrow_state = self.raw as *mut u8; // Set the mutable data borrow bit to `0`; we are guaranteed that account @@ -421,14 +402,6 @@ impl AccountInfo { }) } - /// Checks if it is possible to get a read-only reference to the data field, failing - /// if the field is already mutable borrowed or if 7 borrows already exist. - #[deprecated(since = "0.8.4", note = "Use `can_borrow_data` instead")] - #[inline(always)] - pub fn check_borrow_data(&self) -> Result<(), ProgramError> { - self.can_borrow_data() - } - /// Checks if it is possible to get a read-only reference to the data field, failing /// if the field is already mutable borrowed or if 7 borrows already exist. #[inline(always)] @@ -450,18 +423,10 @@ impl AccountInfo { Ok(()) } - /// Checks if it is possible to get a mutable reference to the data field, failing - /// if the field is already borrowed in any form. - #[deprecated(since = "0.8.4", note = "Use `can_borrow_mut_data` instead")] - #[inline(always)] - pub fn check_borrow_mut_data(&self) -> Result<(), ProgramError> { - self.can_borrow_mut_data() - } - /// Checks if it is possible to get a mutable reference to the data field, failing /// if the field is already borrowed in any form. #[inline(always)] - pub fn can_borrow_mut_data(&self) -> Result<(), ProgramError> { + pub fn can_borrow_data_mut(&self) -> Result<(), ProgramError> { let borrow_state = unsafe { (*self.raw).borrow_state }; // Check whether any (mutable or immutable) data borrow bits are @@ -510,7 +475,7 @@ impl AccountInfo { #[inline] pub fn resize(&self, new_len: usize) -> Result<(), ProgramError> { // Check whether the account data is already borrowed. - self.can_borrow_mut_data()?; + self.can_borrow_data_mut()?; // SAFETY: // We are checking if the account data is already borrowed, so we are safe to call @@ -554,14 +519,7 @@ impl AccountInfo { if difference > 0 { unsafe { - #[cfg(target_os = "solana")] - sol_memset_( - self.data_ptr().add(current_len as usize), - 0, - difference as u64, - ); - #[cfg(not(target_os = "solana"))] - core::ptr::write_bytes( + write_bytes( self.data_ptr().add(current_len as usize), 0, difference as usize, @@ -637,11 +595,11 @@ impl AccountInfo { // - 8 bytes for the data_len // // So we can zero out them directly. - #[cfg(target_os = "solana")] - sol_memset_(self.data_ptr().sub(48), 0, 48); + write_bytes(self.data_ptr().sub(48), 0, 48); } /// Returns the memory address of the account data. + /// /// # Important /// /// Obtaining the raw pointer itself is safe, but de-referencing it requires @@ -849,11 +807,11 @@ impl Drop for RefMut<'_, T> { #[cfg(test)] mod tests { - use core::mem::{size_of, MaybeUninit}; - - use crate::entrypoint::NON_DUP_MARKER as NOT_BORROWED; - - use super::*; + use { + super::*, + core::mem::{size_of, MaybeUninit}, + solana_program_entrypoint::NON_DUP_MARKER as NOT_BORROWED, + }; #[test] fn test_data_ref() { @@ -999,18 +957,18 @@ mod tests { let mut data = [0u64; size_of::() / size_of::() + 1]; // extra byte at end for account data // Set the borrow state. data[0] = NOT_BORROWED as u64; - let account_info = AccountInfo { + let account_view = AccountView { raw: data.as_mut_ptr() as *mut Account, }; // Check that we can borrow data and lamports. - assert!(account_info.can_borrow_data().is_ok()); - assert!(account_info.can_borrow_mut_data().is_ok()); - assert!(account_info.can_borrow_lamports().is_ok()); - assert!(account_info.can_borrow_mut_lamports().is_ok()); + assert!(account_view.can_borrow_data().is_ok()); + assert!(account_view.can_borrow_data_mut().is_ok()); + assert!(account_view.can_borrow_lamports().is_ok()); + assert!(account_view.can_borrow_lamports_mut().is_ok()); // It should be sound to mutate the data through the data pointer while no other borrows exist - let data_ptr = account_info.data_ptr(); + let data_ptr = account_view.data_ptr(); unsafe { let data = core::slice::from_raw_parts_mut(data_ptr, 1); // Data is 1 byte long! data[0] = 1; @@ -1021,20 +979,20 @@ mod tests { let mut refs = [ACCOUNT_REF; 7]; refs.iter_mut().for_each(|r| { - let Ok(data_ref) = account_info.try_borrow_data() else { + let Ok(data_ref) = account_view.try_borrow_data() else { panic!("Failed to borrow data"); }; r.write(data_ref); }); // Check that we cannot borrow the data anymore. - assert!(account_info.can_borrow_data().is_err()); - assert!(account_info.try_borrow_data().is_err()); - assert!(account_info.can_borrow_mut_data().is_err()); - assert!(account_info.try_borrow_mut_data().is_err()); + assert!(account_view.can_borrow_data().is_err()); + assert!(account_view.try_borrow_data().is_err()); + assert!(account_view.can_borrow_data_mut().is_err()); + assert!(account_view.try_borrow_data_mut().is_err()); // Lamports should still be borrowable. - assert!(account_info.can_borrow_lamports().is_ok()); - assert!(account_info.can_borrow_mut_lamports().is_ok()); + assert!(account_view.can_borrow_lamports().is_ok()); + assert!(account_view.can_borrow_lamports_mut().is_ok()); // Drop the immutable borrows. refs.iter_mut().for_each(|r| { @@ -1043,27 +1001,27 @@ mod tests { }); // We should be able to borrow the data again. - assert!(account_info.can_borrow_data().is_ok()); - assert!(account_info.can_borrow_mut_data().is_ok()); + assert!(account_view.can_borrow_data().is_ok()); + assert!(account_view.can_borrow_data_mut().is_ok()); // Borrow mutable data. - let ref_mut = account_info.try_borrow_mut_data().unwrap(); + let ref_mut = account_view.try_borrow_data_mut().unwrap(); // It should be sound to get the data pointer while the data is borrowed as long as we don't use it - let _data_ptr = account_info.data_ptr(); + let _data_ptr = account_view.data_ptr(); // Check that we cannot borrow the data anymore. - assert!(account_info.can_borrow_data().is_err()); - assert!(account_info.try_borrow_data().is_err()); - assert!(account_info.can_borrow_mut_data().is_err()); - assert!(account_info.try_borrow_mut_data().is_err()); + assert!(account_view.can_borrow_data().is_err()); + assert!(account_view.try_borrow_data().is_err()); + assert!(account_view.can_borrow_data_mut().is_err()); + assert!(account_view.try_borrow_data_mut().is_err()); drop(ref_mut); // We should be able to borrow the data again. - assert!(account_info.can_borrow_data().is_ok()); - assert!(account_info.can_borrow_mut_data().is_ok()); + assert!(account_view.can_borrow_data().is_ok()); + assert!(account_view.can_borrow_data_mut().is_ok()); - let borrow_state = unsafe { (*account_info.raw).borrow_state }; + let borrow_state = unsafe { (*account_view.raw).borrow_state }; assert!(borrow_state == NOT_BORROWED); } @@ -1073,35 +1031,35 @@ mod tests { let mut data = [0u64; size_of::() / size_of::()]; // Set the borrow state. data[0] = NOT_BORROWED as u64; - let account_info = AccountInfo { + let account_view = AccountView { raw: data.as_mut_ptr() as *mut Account, }; // Check that we can borrow lamports and data. - assert!(account_info.can_borrow_lamports().is_ok()); - assert!(account_info.can_borrow_mut_lamports().is_ok()); - assert!(account_info.can_borrow_data().is_ok()); - assert!(account_info.can_borrow_mut_data().is_ok()); + assert!(account_view.can_borrow_lamports().is_ok()); + assert!(account_view.can_borrow_lamports_mut().is_ok()); + assert!(account_view.can_borrow_data().is_ok()); + assert!(account_view.can_borrow_data_mut().is_ok()); // Borrow immutable lamports (7 immutable borrows available). const LAMPORTS_REF: MaybeUninit> = MaybeUninit::>::uninit(); let mut refs = [LAMPORTS_REF; 7]; refs.iter_mut().for_each(|r| { - let Ok(lamports_ref) = account_info.try_borrow_lamports() else { + let Ok(lamports_ref) = account_view.try_borrow_lamports() else { panic!("Failed to borrow lamports"); }; r.write(lamports_ref); }); // Check that we cannot borrow the lamports anymore. - assert!(account_info.can_borrow_lamports().is_err()); - assert!(account_info.try_borrow_lamports().is_err()); - assert!(account_info.can_borrow_mut_lamports().is_err()); - assert!(account_info.try_borrow_mut_lamports().is_err()); + assert!(account_view.can_borrow_lamports().is_err()); + assert!(account_view.try_borrow_lamports().is_err()); + assert!(account_view.can_borrow_lamports_mut().is_err()); + assert!(account_view.try_borrow_lamports_mut().is_err()); // Data should still be borrowable. - assert!(account_info.can_borrow_data().is_ok()); - assert!(account_info.can_borrow_mut_data().is_ok()); + assert!(account_view.can_borrow_data().is_ok()); + assert!(account_view.can_borrow_data_mut().is_ok()); // Drop the immutable borrows. refs.iter_mut().for_each(|r| { @@ -1110,25 +1068,25 @@ mod tests { }); // We should be able to borrow the lamports again. - assert!(account_info.can_borrow_lamports().is_ok()); - assert!(account_info.can_borrow_mut_lamports().is_ok()); + assert!(account_view.can_borrow_lamports().is_ok()); + assert!(account_view.can_borrow_lamports_mut().is_ok()); // Borrow mutable lamports. - let ref_mut = account_info.try_borrow_mut_lamports().unwrap(); + let ref_mut = account_view.try_borrow_lamports_mut().unwrap(); // Check that we cannot borrow the lamports anymore. - assert!(account_info.can_borrow_lamports().is_err()); - assert!(account_info.try_borrow_lamports().is_err()); - assert!(account_info.can_borrow_mut_lamports().is_err()); - assert!(account_info.try_borrow_mut_lamports().is_err()); + assert!(account_view.can_borrow_lamports().is_err()); + assert!(account_view.try_borrow_lamports().is_err()); + assert!(account_view.can_borrow_lamports_mut().is_err()); + assert!(account_view.try_borrow_lamports_mut().is_err()); drop(ref_mut); // We should be able to borrow the data again. - assert!(account_info.can_borrow_lamports().is_ok()); - assert!(account_info.can_borrow_mut_lamports().is_ok()); + assert!(account_view.can_borrow_lamports().is_ok()); + assert!(account_view.can_borrow_lamports_mut().is_ok()); - let borrow_state = unsafe { (*account_info.raw).borrow_state }; + let borrow_state = unsafe { (*account_view.raw).borrow_state }; assert!(borrow_state == NOT_BORROWED); } @@ -1144,7 +1102,7 @@ mod tests { // - index `10` is equal to offset `10 * size_of::() = 80` bytes. data[10] = 100; - let account = AccountInfo { + let account = AccountView { raw: data.as_mut_ptr() as *const _ as *mut Account, }; From 80aa9ebaf7d4ed823c66f6010230cf0ffedf37c0 Mon Sep 17 00:00:00 2001 From: febo Date: Mon, 22 Sep 2025 15:44:21 +0100 Subject: [PATCH 40/51] Improve account borrows --- Cargo.lock | 1 - account-view/Cargo.toml | 3 - account-view/src/lib.rs | 569 +++++++++------------------------------- 3 files changed, 130 insertions(+), 443 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f426c77c..3ccf64bef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2761,7 +2761,6 @@ name = "solana-account-view" version = "0.0.0" dependencies = [ "solana-address", - "solana-program-entrypoint", "solana-program-error", ] diff --git a/account-view/Cargo.toml b/account-view/Cargo.toml index 0088a2316..691695994 100644 --- a/account-view/Cargo.toml +++ b/account-view/Cargo.toml @@ -14,8 +14,5 @@ edition = { workspace = true } solana-address = { workspace = true } solana-program-error = { workspace = true } -[dev-dependencies] -solana-program-entrypoint = { workspace = true } - [lints] workspace = true diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 05f332572..76867d591 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -5,7 +5,8 @@ use { core::{ marker::PhantomData, - mem::ManuallyDrop, + mem::{size_of, ManuallyDrop}, + ops::{Deref, DerefMut}, ptr::{write, write_bytes, NonNull}, slice::{from_raw_parts, from_raw_parts_mut}, }, @@ -17,24 +18,8 @@ use { /// single top-level instruction. pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; -/// Represents masks for borrow state of an account. -#[repr(u8)] -#[derive(Clone, Copy, Debug)] -pub enum BorrowState { - /// Mask to check whether an account is already borrowed. - /// - /// This will test both data and lamports borrow state. Any position - /// in the borrow byte that is not set means that the account - /// is borrowed in that state. - Borrowed = 0b_1111_1111, - - /// Mask to check whether an account is already mutably borrowed. - /// - /// This will test both data and lamports mutable borrow state. If - /// one of the mutably borrowed bits is not set, then the account - /// is mutably borrowed in that state. - MutablyBorrowed = 0b_1000_1000, -} +/// Value to indicate that an account is not borrowed. +pub const NOT_BORROWED: u8 = u8::MAX; /// Raw account data. /// @@ -48,35 +33,11 @@ pub struct Account { /// Borrow state for lamports and account data. /// /// This reuses the memory reserved for the duplicate flag in the - /// account to track lamports and data borrows. It represents the - /// numbers of borrows available. - /// - /// Bits in the borrow byte are used as follows: - /// - /// * lamport mutable borrow flag - /// - `7 6 5 4 3 2 1 0` - /// - `x . . . . . . .`: `1` - the lamport field can be mutably borrowed; - /// `0` - there is an outstanding mutable borrow for the lamports. - /// - /// * lamport immutable borrow count - /// - `7 6 5 4 3 2 1 0` - /// - `. x x x . . . .`: number of immutable borrows that can still be - /// allocated, for the lamports field. Ranges from 7 (`111`) to - /// 0 (`000`). - /// - /// * data mutable borrow flag - /// - `7 6 5 4 3 2 1 0` - /// - `. . . . x . . .`: `1` - the account data can be mutably borrowed; - /// `0` - there is an outstanding mutable borrow for the account data. - /// - /// * data immutable borrow count - /// - `7 6 5 4 3 2 1 0` - /// - `. . . . . x x x`: Number of immutable borrows that can still be - /// allocated, for the account data. Ranges from 7 (`111`) to 0 (`000`). - /// - /// Note that this values are shared across `AccountInfo`s over the - /// same account, e.g., in case of duplicated accounts, they share - /// the same borrow state. + /// account to track data borrows. It represents the numbers of + /// borrows available. The value `0` indicates that the account + /// data is mutably borrowed, while values between `2` and `255` + /// indicate the number of immutable borrows that can still be + /// allocated. pub borrow_state: u8, /// Indicates whether the transaction was signed by this account. @@ -135,72 +96,99 @@ pub struct AccountView { } impl AccountView { - /// Public key of the account. + /// Address of the account. #[inline(always)] - pub fn key(&self) -> &Address { + pub const fn key(&self) -> &Address { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { &(*self.raw).key } } - /// Program that owns this account. + /// Return a reference to the address of the program that owns this account. + /// + /// For ownership checks, use the safe `owned_by` method instead. + /// + /// # Safety + /// + /// This method is unsafe because it returns a reference to the owner field, + /// which can be modified by `assign` and `close` methods. It is undefined + /// behavior to use this reference after the account has been modified. #[inline(always)] - pub fn owner(&self) -> &Address { + pub const unsafe fn owner(&self) -> &Address { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { &(*self.raw).owner } } - /// Indicates whether the transaction was signed by this account. + /// Indicate whether the transaction was signed by this account. #[inline(always)] - pub fn is_signer(&self) -> bool { + pub const fn is_signer(&self) -> bool { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).is_signer != 0 } } - /// Indicates whether the account is writable. + /// Indicate whether the account is writable. #[inline(always)] - pub fn is_writable(&self) -> bool { + pub const fn is_writable(&self) -> bool { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).is_writable != 0 } } - /// Indicates whether this account represents a program. + /// Indicate whether this account represents an executable program. /// /// Program accounts are always read-only. #[inline(always)] - pub fn executable(&self) -> bool { + pub const fn executable(&self) -> bool { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).executable != 0 } } - /// Returns the size of the data in the account. + /// Return the size of the data in the account. #[inline(always)] - pub fn data_len(&self) -> usize { + pub const fn data_len(&self) -> usize { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).data_len as usize } } - /// Returns the delta between the original data length and the current + /// Return the delta between the original data length and the current /// data length. /// - /// This value will be different than zero if the account has been resized - /// during the current instruction. + /// This value will be different than zero if the account has been + /// resized during the current instruction. #[inline(always)] - pub fn resize_delta(&self) -> i32 { + pub const fn resize_delta(&self) -> i32 { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).resize_delta } } - /// Returns the lamports in the account. + /// Return the lamports in the account. #[inline(always)] - pub fn lamports(&self) -> u64 { + pub const fn lamports(&self) -> u64 { + // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).lamports } } + /// Set the lamports in the account. + #[inline(always)] + pub const fn set_lamports(&self, lamports: u64) { + // SAFETY: The `raw` pointer is guaranteed to be valid. + unsafe { + (*self.raw).lamports = lamports; + } + } + /// Indicates whether the account data is empty. /// /// An account is considered empty if the data length is zero. #[inline(always)] - pub fn is_data_empty(&self) -> bool { + pub const fn is_data_empty(&self) -> bool { + // SAFETY: The `raw` pointer is guaranteed to be valid. self.data_len() == 0 } /// Checks if the account is owned by the given program. #[inline(always)] pub fn owned_by(&self, program: &Address) -> bool { - self.owner() == program + // SAFETY: The `raw` pointer is guaranteed to be valid. + unsafe { self.owner() == program } } /// Changes the owner of the account. @@ -214,39 +202,16 @@ impl AccountView { write(&mut (*self.raw).owner, *new_owner); } - /// Return true if the account borrow state is set to the given state. - /// - /// This will test both data and lamports borrow state. + /// Return `true` if the account data is borrowed in any form. #[inline(always)] - pub fn is_borrowed(&self, state: BorrowState) -> bool { - let borrow_state = unsafe { (*self.raw).borrow_state }; - let mask = state as u8; - // If borrow state has any of the state bits of the mask not set, - // then the account is borrowed for that state. - (borrow_state & mask) != mask + pub fn is_borrowed(&self) -> bool { + unsafe { (*self.raw).borrow_state != NOT_BORROWED } } - /// Returns a read-only reference to the lamports in the account. - /// - /// # Safety - /// - /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow - /// flag untouched. Useful when an instruction has verified non-duplicate accounts. + /// Return `true` if the account data is mutably borrowed. #[inline(always)] - pub unsafe fn borrow_lamports_unchecked(&self) -> &u64 { - &(*self.raw).lamports - } - - /// Returns a mutable reference to the lamports in the account. - /// - /// # Safety - /// - /// This method is unsafe because it does not return a `Ref`, thus leaving the borrow - /// flag untouched. Useful when an instruction has verified non-duplicate accounts. - #[allow(clippy::mut_from_ref)] - #[inline(always)] - pub unsafe fn borrow_lamports_unchecked_mut(&self) -> &mut u64 { - &mut (*self.raw).lamports + pub fn is_borrowed_mut(&self) -> bool { + unsafe { (*self.raw).borrow_state == 0 } } /// Returns a read-only reference to the data in the account. @@ -272,89 +237,6 @@ impl AccountView { from_raw_parts_mut(self.data_ptr(), self.data_len()) } - /// Tries to get a read-only reference to the lamport field, failing if the - /// field is already mutable borrowed or if 7 borrows already exist. - pub fn try_borrow_lamports(&self) -> Result, ProgramError> { - // check if the account lamports are already borrowed - self.can_borrow_lamports()?; - - let borrow_state = self.raw as *mut u8; - // Use one immutable borrow for lamports by subtracting `1` from the - // lamports borrow counter bits; we are guaranteed that there is at - // least one immutable borrow available. - // - // SAFETY: The `borrow_state` is a mutable pointer to the borrow state - // of the account, which is guaranteed to be valid. - unsafe { *borrow_state -= 1 << LAMPORTS_BORROW_SHIFT }; - - // return the reference to lamports - Ok(Ref { - value: unsafe { NonNull::from(&(*self.raw).lamports) }, - state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_shift: LAMPORTS_BORROW_SHIFT, - marker: PhantomData, - }) - } - - /// Tries to get a read only reference to the lamport field, failing if the field - /// is already borrowed in any form. - pub fn try_borrow_lamports_mut(&self) -> Result, ProgramError> { - // check if the account lamports are already borrowed - self.can_borrow_lamports_mut()?; - - let borrow_state = self.raw as *mut u8; - // Set the mutable lamports borrow bit to `0`; we are guaranteed - // that lamports are not already borrowed in any form. - // - // SAFETY: The `borrow_state` is a mutable pointer to the borrow state - // of the account, which is guaranteed to be valid. - unsafe { *borrow_state &= 0b_0111_1111 }; - - // return the mutable reference to lamports - Ok(RefMut { - value: unsafe { NonNull::from(&mut (*self.raw).lamports) }, - state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_bitmask: LAMPORTS_MUTABLE_BORROW_BITMASK, - marker: PhantomData, - }) - } - - /// Checks if it is possible to get a read-only reference to the lamport field, - /// failing if the field is already mutable borrowed or if `7` borrows already exist. - #[inline(always)] - pub fn can_borrow_lamports(&self) -> Result<(), ProgramError> { - let borrow_state = unsafe { (*self.raw).borrow_state }; - - // Check whether the mutable lamports borrow bit is already in - // use (value `0`) or not. If it is `0`, then the borrow will fail. - if borrow_state & LAMPORTS_MUTABLE_BORROW_BITMASK == 0 { - return Err(ProgramError::AccountBorrowFailed); - } - - // Check whether we have reached the maximum immutable lamports borrow count - // or not, i.e., it fails when all immutable lamports borrow bits are `0`. - if borrow_state & 0b_0111_0000 == 0 { - return Err(ProgramError::AccountBorrowFailed); - } - - Ok(()) - } - - /// Checks if it is possible to get a mutable reference to the lamport field, - /// failing if the field is already borrowed in any form. - #[inline(always)] - pub fn can_borrow_lamports_mut(&self) -> Result<(), ProgramError> { - let borrow_state = unsafe { (*self.raw).borrow_state }; - - // Check whether any (mutable or immutable) lamports borrow bits are - // in use (value `0`) or not. - if borrow_state & 0b_1111_0000 != 0b_1111_0000 { - return Err(ProgramError::AccountBorrowFailed); - } - - Ok(()) - } - /// Tries to get a read-only reference to the data field, failing if the field /// is already mutable borrowed or if `7` borrows already exist. pub fn try_borrow_data(&self) -> Result, ProgramError> { @@ -374,7 +256,6 @@ impl AccountView { Ok(Ref { value: unsafe { NonNull::from(from_raw_parts(self.data_ptr(), self.data_len())) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_shift: DATA_BORROW_SHIFT, marker: PhantomData, }) } @@ -391,76 +272,43 @@ impl AccountView { // // SAFETY: The `borrow_state` is a mutable pointer to the borrow state // of the account, which is guaranteed to be valid. - unsafe { *borrow_state &= 0b_1111_0111 }; + unsafe { *borrow_state = 0 }; // return the mutable reference to data Ok(RefMut { value: unsafe { NonNull::from(from_raw_parts_mut(self.data_ptr(), self.data_len())) }, state: unsafe { NonNull::new_unchecked(borrow_state) }, - borrow_bitmask: DATA_MUTABLE_BORROW_BITMASK, marker: PhantomData, }) } - /// Checks if it is possible to get a read-only reference to the data field, failing - /// if the field is already mutable borrowed or if 7 borrows already exist. + /// Check if it is possible to get a immutable reference to the data field, + /// failing if the field is already mutably borrowed or there are not enough + /// immutable borrows available. #[inline(always)] pub fn can_borrow_data(&self) -> Result<(), ProgramError> { - let borrow_state = unsafe { (*self.raw).borrow_state }; - - // Check whether the mutable data borrow bit is already in - // use (value `0`) or not. If it is `0`, then the borrow will fail. - if borrow_state & DATA_MUTABLE_BORROW_BITMASK == 0 { - return Err(ProgramError::AccountBorrowFailed); - } - - // Check whether we have reached the maximum immutable data borrow count - // or not, i.e., it fails when all immutable data borrow bits are `0`. - if borrow_state & 0b_0000_0111 == 0 { + // There must be at least one immutable borrow available. + // + // SAFETY: The `raw` pointer is guaranteed to be valid. + if unsafe { (*self.raw).borrow_state } < 2 { return Err(ProgramError::AccountBorrowFailed); } Ok(()) } - /// Checks if it is possible to get a mutable reference to the data field, failing - /// if the field is already borrowed in any form. + /// Checks if it is possible to get a mutable reference to the data field, + /// failing if the field is already borrowed in any form. #[inline(always)] pub fn can_borrow_data_mut(&self) -> Result<(), ProgramError> { - let borrow_state = unsafe { (*self.raw).borrow_state }; - - // Check whether any (mutable or immutable) data borrow bits are - // in use (value `0`) or not. - if borrow_state & 0b_0000_1111 != 0b_0000_1111 { + // SAFETY: The `raw` pointer is guaranteed to be valid. + if unsafe { (*self.raw).borrow_state } != NOT_BORROWED { return Err(ProgramError::AccountBorrowFailed); } Ok(()) } - /// Realloc (either truncating or zero extending) the account's data. - /// - /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes - /// within an instruction. - /// - /// # Important - /// - /// The use of the `zero_init` parameter, which indicated whether the newly - /// allocated memory should be zero-initialized or not, is now deprecated and - /// ignored. The method will always zero-initialize the newly allocated memory - /// if the new length is larger than the current data length. This is the same - /// behavior as [`Self::resize`]. - /// - /// This method makes assumptions about the layout and location of memory - /// referenced by `AccountInfo` fields. It should only be called for - /// instances of `AccountInfo` that were created by the runtime and received - /// in the `process_instruction` entrypoint of a program. - #[deprecated(since = "0.9.0", note = "Use AccountInfo::resize() instead")] - #[inline(always)] - pub fn realloc(&self, new_len: usize, _zero_init: bool) -> Result<(), ProgramError> { - self.resize(new_len) - } - /// Resize (either truncating or zero extending) the account's data. /// /// The account data can be increased by up to [`MAX_PERMITTED_DATA_INCREASE`] bytes @@ -469,16 +317,16 @@ impl AccountView { /// # Important /// /// This method makes assumptions about the layout and location of memory - /// referenced by `AccountInfo` fields. It should only be called for - /// instances of `AccountInfo` that were created by the runtime and received - /// in the `process_instruction` entrypoint of a program. + /// referenced by `Account` fields. It should only be called for instances + /// of `AccountView` that were created by the runtime and received in the + /// `process_instruction` entrypoint of a program. #[inline] pub fn resize(&self, new_len: usize) -> Result<(), ProgramError> { // Check whether the account data is already borrowed. self.can_borrow_data_mut()?; - // SAFETY: - // We are checking if the account data is already borrowed, so we are safe to call + // SAFETY: We are checking if the account data is already borrowed, so + // we are safe to call. unsafe { self.resize_unchecked(new_len) } } @@ -543,9 +391,9 @@ impl AccountView { /// an unbalanced instruction error. #[inline] pub fn close(&self) -> ProgramResult { - // make sure the account is not borrowed since we are about to - // resize the data to zero - if self.is_borrowed(BorrowState::Borrowed) { + // Make sure the account is not borrowed since we are about to + // resize the data to zero. + if self.is_borrowed() { return Err(ProgramError::AccountBorrowFailed); } @@ -603,34 +451,22 @@ impl AccountView { /// # Important /// /// Obtaining the raw pointer itself is safe, but de-referencing it requires - /// the caller to uphold Rust's aliasing rules. It is undefined behavior to de-reference - /// the pointer or write through it while any safe reference (e.g., from any of `borrow_data` - /// or `borrow_mut_data` methods) to the same data is still alive. + /// the caller to uphold Rust's aliasing rules. It is undefined behavior to + /// de-reference the pointer or write through it while any safe reference + /// (e.g., from any of `borrow_data` or `borrow_mut_data` methods) to the same + /// data is still alive. + #[inline(always)] pub fn data_ptr(&self) -> *mut u8 { - unsafe { (self.raw as *mut u8).add(core::mem::size_of::()) } + // SAFETY: The `raw` pointer is guaranteed to be valid. + unsafe { (self.raw as *mut u8).add(size_of::()) } } } -/// Number of bits of the [`Account::borrow_state`] flag to shift to get to -/// the borrow state bits for lamports. -/// - `7 6 5 4 3 2 1 0` -/// - `x x x x . . . .` -const LAMPORTS_BORROW_SHIFT: u8 = 4; - -/// Number of bits of the [`Account::borrow_state`] flag to shift to get to -/// the borrow state bits for account data. -/// - `7 6 5 4 3 2 1 0` -/// - `. . . . x x x x` -const DATA_BORROW_SHIFT: u8 = 0; - /// Reference to account data or lamports with checked borrow rules. #[derive(Debug)] pub struct Ref<'a, T: ?Sized> { value: NonNull, state: NonNull, - /// Indicates the type of borrow (lamports or data) by representing the - /// shift amount. - borrow_shift: u8, /// The `value` raw pointer is only valid while the `&'a T` lives so we claim /// to hold a reference to it. marker: PhantomData<&'a T>, @@ -648,7 +484,6 @@ impl<'a, T: ?Sized> Ref<'a, T> { Ref { value: NonNull::from(f(&*orig)), state: orig.state, - borrow_shift: orig.borrow_shift, marker: PhantomData, } } @@ -667,7 +502,6 @@ impl<'a, T: ?Sized> Ref<'a, T> { Ok(value) => Ok(Ref { value: NonNull::from(value), state: orig.state, - borrow_shift: orig.borrow_shift, marker: PhantomData, }), Err(e) => Err((ManuallyDrop::into_inner(orig), e)), @@ -687,7 +521,6 @@ impl<'a, T: ?Sized> Ref<'a, T> { Some(value) => Ok(Ref { value: NonNull::from(value), state: orig.state, - borrow_shift: orig.borrow_shift, marker: PhantomData, }), None => Err(ManuallyDrop::into_inner(orig)), @@ -695,7 +528,7 @@ impl<'a, T: ?Sized> Ref<'a, T> { } } -impl core::ops::Deref for Ref<'_, T> { +impl Deref for Ref<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { unsafe { self.value.as_ref() } @@ -703,26 +536,17 @@ impl core::ops::Deref for Ref<'_, T> { } impl Drop for Ref<'_, T> { - // decrement the immutable borrow count fn drop(&mut self) { - unsafe { *self.state.as_mut() += 1 << self.borrow_shift }; + // Increment the available borrow count. + unsafe { *self.state.as_mut() += 1 }; } } -/// Mask representing the mutable borrow flag for lamports. -const LAMPORTS_MUTABLE_BORROW_BITMASK: u8 = 0b_1000_0000; - -/// Mask representing the mutable borrow flag for data. -const DATA_MUTABLE_BORROW_BITMASK: u8 = 0b_0000_1000; - /// Mutable reference to account data or lamports with checked borrow rules. #[derive(Debug)] pub struct RefMut<'a, T: ?Sized> { value: NonNull, state: NonNull, - /// Indicates borrowed field (lamports or data) by storing the bitmask - /// representing the mutable borrow. - borrow_bitmask: u8, /// The `value` raw pointer is only valid while the `&'a T` lives so we claim /// to hold a reference to it. marker: PhantomData<&'a mut T>, @@ -740,7 +564,6 @@ impl<'a, T: ?Sized> RefMut<'a, T> { RefMut { value: NonNull::from(f(&mut *orig)), state: orig.state, - borrow_bitmask: orig.borrow_bitmask, marker: PhantomData, } } @@ -759,7 +582,6 @@ impl<'a, T: ?Sized> RefMut<'a, T> { Ok(value) => Ok(RefMut { value: NonNull::from(value), state: orig.state, - borrow_bitmask: orig.borrow_bitmask, marker: PhantomData, }), Err(e) => Err((ManuallyDrop::into_inner(orig), e)), @@ -778,7 +600,6 @@ impl<'a, T: ?Sized> RefMut<'a, T> { Some(value) => Ok(RefMut { value: NonNull::from(value), state: orig.state, - borrow_bitmask: orig.borrow_bitmask, marker: PhantomData, }), None => Err(ManuallyDrop::into_inner(orig)), @@ -786,13 +607,13 @@ impl<'a, T: ?Sized> RefMut<'a, T> { } } -impl core::ops::Deref for RefMut<'_, T> { +impl Deref for RefMut<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { unsafe { self.value.as_ref() } } } -impl core::ops::DerefMut for RefMut<'_, T> { +impl DerefMut for RefMut<'_, T> { fn deref_mut(&mut self) -> &mut ::Target { unsafe { self.value.as_mut() } } @@ -800,8 +621,8 @@ impl core::ops::DerefMut for RefMut<'_, T> { impl Drop for RefMut<'_, T> { fn drop(&mut self) { - // unset the mutable borrow flag - unsafe { *self.state.as_mut() |= self.borrow_bitmask }; + // Reset the borrow state. + unsafe { *self.state.as_mut() = NOT_BORROWED }; } } @@ -810,17 +631,15 @@ mod tests { use { super::*, core::mem::{size_of, MaybeUninit}, - solana_program_entrypoint::NON_DUP_MARKER as NOT_BORROWED, }; #[test] fn test_data_ref() { let data: [u8; 4] = [0, 1, 2, 3]; - let mut state = NOT_BORROWED - (1 << DATA_BORROW_SHIFT); + let mut state = NOT_BORROWED - 1; let ref_data = Ref { value: NonNull::from(&data), - borrow_shift: DATA_BORROW_SHIFT, // borrow state must be a mutable reference state: NonNull::from(&mut state), marker: PhantomData, @@ -828,67 +647,32 @@ mod tests { let new_ref = Ref::map(ref_data, |data| &data[1]); - assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); + assert_eq!(state, NOT_BORROWED - 1); assert_eq!(*new_ref, 1); let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&3)) else { unreachable!() }; - assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); + assert_eq!(state, NOT_BORROWED - 1); assert_eq!(*new_ref, 3); let Ok(new_ref) = Ref::try_map::<_, u8>(new_ref, |_| Ok(&4)) else { unreachable!() }; - assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); + assert_eq!(state, NOT_BORROWED - 1); assert_eq!(*new_ref, 4); let (new_ref, err) = Ref::try_map::(new_ref, |_| Err(5)).unwrap_err(); - assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); + assert_eq!(state, NOT_BORROWED - 1); assert_eq!(err, 5); // Unchanged assert_eq!(*new_ref, 4); let new_ref = Ref::filter_map(new_ref, |_| Option::<&u8>::None); - assert_eq!(state, NOT_BORROWED - (1 << DATA_BORROW_SHIFT)); - assert!(new_ref.is_err()); - - drop(new_ref); - - assert_eq!(state, NOT_BORROWED); - } - - #[test] - fn test_lamports_ref() { - let lamports: u64 = 10000; - let mut state = NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT); - - let ref_lamports = Ref { - value: NonNull::from(&lamports), - borrow_shift: LAMPORTS_BORROW_SHIFT, - // borrow state must be a mutable reference - state: NonNull::from(&mut state), - marker: PhantomData, - }; - - let new_ref = Ref::map(ref_lamports, |_| &1000); - - assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT)); - assert_eq!(*new_ref, 1000); - - let Ok(new_ref) = Ref::filter_map(new_ref, |_| Some(&2000)) else { - unreachable!() - }; - - assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT)); - assert_eq!(*new_ref, 2000); - - let new_ref = Ref::filter_map(new_ref, |_| Option::<&i32>::None); - - assert_eq!(state, NOT_BORROWED - (1 << LAMPORTS_BORROW_SHIFT)); + assert_eq!(state, NOT_BORROWED - 1); assert!(new_ref.is_err()); drop(new_ref); @@ -899,11 +683,10 @@ mod tests { #[test] fn test_data_ref_mut() { let mut data: [u8; 4] = [0, 1, 2, 3]; - let mut state = 0b_1111_0111; + let mut state = 0; let ref_data = RefMut { value: NonNull::from(&mut data), - borrow_bitmask: DATA_MUTABLE_BORROW_BITMASK, // borrow state must be a mutable reference state: NonNull::from(&mut state), marker: PhantomData, @@ -915,7 +698,7 @@ mod tests { *new_ref = 4; - assert_eq!(state, 0b_1111_0111); + assert_eq!(state, 0); assert_eq!(*new_ref, 4); drop(new_ref); @@ -924,64 +707,40 @@ mod tests { assert_eq!(state, NOT_BORROWED); } - #[test] - fn test_lamports_ref_mut() { - let mut lamports: u64 = 10000; - let mut state = 0b_0111_1111; - - let ref_lamports = RefMut { - value: NonNull::from(&mut lamports), - borrow_bitmask: LAMPORTS_MUTABLE_BORROW_BITMASK, - // borrow state must be a mutable reference - state: NonNull::from(&mut state), - marker: PhantomData, - }; - - let new_ref = RefMut::map(ref_lamports, |lamports| { - *lamports = 200; - lamports - }); - - assert_eq!(state, 0b_0111_1111); - assert_eq!(*new_ref, 200); - - drop(new_ref); - - assert_eq!(lamports, 200); - assert_eq!(state, NOT_BORROWED); - } - #[test] fn test_borrow_data() { - // 8-bytes aligned account data. - let mut data = [0u64; size_of::() / size_of::() + 1]; // extra byte at end for account data - // Set the borrow state. + // 8-bytes aligned account data + 8 bytes of trailing data. + let mut data = [0u64; size_of::() / size_of::() + 1]; + let account = data.as_mut_ptr() as *mut Account; + unsafe { (*account).data_len = 8 }; + data[0] = NOT_BORROWED as u64; - let account_view = AccountView { - raw: data.as_mut_ptr() as *mut Account, - }; + let account_view = AccountView { raw: account }; // Check that we can borrow data and lamports. assert!(account_view.can_borrow_data().is_ok()); assert!(account_view.can_borrow_data_mut().is_ok()); - assert!(account_view.can_borrow_lamports().is_ok()); - assert!(account_view.can_borrow_lamports_mut().is_ok()); - // It should be sound to mutate the data through the data pointer while no other borrows exist + // It should be sound to mutate the data through the data pointer + // while no other borrows exist. let data_ptr = account_view.data_ptr(); unsafe { - let data = core::slice::from_raw_parts_mut(data_ptr, 1); // Data is 1 byte long! + // There are 8 bytes of trailing data. + let data = from_raw_parts_mut(data_ptr, 8); data[0] = 1; } - // Borrow immutable data (7 immutable borrows available). + // Borrow multiple immutable data references (254 immutable borrows + // available). const ACCOUNT_REF: MaybeUninit> = MaybeUninit::>::uninit(); - let mut refs = [ACCOUNT_REF; 7]; + let mut refs = [ACCOUNT_REF; (NOT_BORROWED as usize) - 1]; refs.iter_mut().for_each(|r| { let Ok(data_ref) = account_view.try_borrow_data() else { panic!("Failed to borrow data"); }; + // Sanity check: the data pointer should see the change. + assert!(data_ref[0] == 1); r.write(data_ref); }); @@ -990,9 +749,6 @@ mod tests { assert!(account_view.try_borrow_data().is_err()); assert!(account_view.can_borrow_data_mut().is_err()); assert!(account_view.try_borrow_data_mut().is_err()); - // Lamports should still be borrowable. - assert!(account_view.can_borrow_lamports().is_ok()); - assert!(account_view.can_borrow_lamports_mut().is_ok()); // Drop the immutable borrows. refs.iter_mut().for_each(|r| { @@ -1006,7 +762,8 @@ mod tests { // Borrow mutable data. let ref_mut = account_view.try_borrow_data_mut().unwrap(); - // It should be sound to get the data pointer while the data is borrowed as long as we don't use it + // It should be sound to get the data pointer while the data is borrowed + // as long as we don't use it. let _data_ptr = account_view.data_ptr(); // Check that we cannot borrow the data anymore. @@ -1026,73 +783,7 @@ mod tests { } #[test] - fn test_borrow_lamports() { - // 8-bytes aligned account data. - let mut data = [0u64; size_of::() / size_of::()]; - // Set the borrow state. - data[0] = NOT_BORROWED as u64; - let account_view = AccountView { - raw: data.as_mut_ptr() as *mut Account, - }; - - // Check that we can borrow lamports and data. - assert!(account_view.can_borrow_lamports().is_ok()); - assert!(account_view.can_borrow_lamports_mut().is_ok()); - assert!(account_view.can_borrow_data().is_ok()); - assert!(account_view.can_borrow_data_mut().is_ok()); - - // Borrow immutable lamports (7 immutable borrows available). - const LAMPORTS_REF: MaybeUninit> = MaybeUninit::>::uninit(); - let mut refs = [LAMPORTS_REF; 7]; - - refs.iter_mut().for_each(|r| { - let Ok(lamports_ref) = account_view.try_borrow_lamports() else { - panic!("Failed to borrow lamports"); - }; - r.write(lamports_ref); - }); - - // Check that we cannot borrow the lamports anymore. - assert!(account_view.can_borrow_lamports().is_err()); - assert!(account_view.try_borrow_lamports().is_err()); - assert!(account_view.can_borrow_lamports_mut().is_err()); - assert!(account_view.try_borrow_lamports_mut().is_err()); - // Data should still be borrowable. - assert!(account_view.can_borrow_data().is_ok()); - assert!(account_view.can_borrow_data_mut().is_ok()); - - // Drop the immutable borrows. - refs.iter_mut().for_each(|r| { - let r = unsafe { r.assume_init_read() }; - drop(r); - }); - - // We should be able to borrow the lamports again. - assert!(account_view.can_borrow_lamports().is_ok()); - assert!(account_view.can_borrow_lamports_mut().is_ok()); - - // Borrow mutable lamports. - let ref_mut = account_view.try_borrow_lamports_mut().unwrap(); - - // Check that we cannot borrow the lamports anymore. - assert!(account_view.can_borrow_lamports().is_err()); - assert!(account_view.try_borrow_lamports().is_err()); - assert!(account_view.can_borrow_lamports_mut().is_err()); - assert!(account_view.try_borrow_lamports_mut().is_err()); - - drop(ref_mut); - - // We should be able to borrow the data again. - assert!(account_view.can_borrow_lamports().is_ok()); - assert!(account_view.can_borrow_lamports_mut().is_ok()); - - let borrow_state = unsafe { (*account_view.raw).borrow_state }; - assert!(borrow_state == NOT_BORROWED); - } - - #[test] - #[allow(deprecated)] - fn test_realloc() { + fn test_resize() { // 8-bytes aligned account data. let mut data = [0u64; 100 * size_of::()]; @@ -1114,7 +805,7 @@ mod tests { // increase the size. - account.realloc(200, false).unwrap(); + account.resize(200).unwrap(); let data_ptr_after = account.data_ptr(); // The data pointer should point to the same address regardless of the reallocation @@ -1125,28 +816,28 @@ mod tests { // decrease the size. - account.realloc(0, false).unwrap(); + account.resize(0).unwrap(); assert_eq!(account.data_len(), 0); assert_eq!(account.resize_delta(), -100); // Invalid reallocation. - let invalid_realloc = account.realloc(10_000_000_001, false); + let invalid_realloc = account.resize(10_000_000_001); assert!(invalid_realloc.is_err()); // Reset to its original size. - account.realloc(100, false).unwrap(); + account.resize(100).unwrap(); assert_eq!(account.data_len(), 100); assert_eq!(account.resize_delta(), 0); - // Consecutive reallocations. + // Consecutive resizes. - account.realloc(200, false).unwrap(); - account.realloc(50, false).unwrap(); - account.realloc(500, false).unwrap(); + account.resize(200).unwrap(); + account.resize(50).unwrap(); + account.resize(500).unwrap(); assert_eq!(account.data_len(), 500); assert_eq!(account.resize_delta(), 400); From e0abc8b9c0d51f2bc769f06f96564a644af7aa46 Mon Sep 17 00:00:00 2001 From: febo Date: Tue, 23 Sep 2025 21:50:55 -0300 Subject: [PATCH 41/51] Add constructor --- account-view/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 76867d591..ca7819260 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -96,6 +96,18 @@ pub struct AccountView { } impl AccountView { + /// Creates a new [`AccountView`] for a given raw account pointer. + /// + /// # Safety + /// + /// The caller must ensure that the `raw` pointer is valid and points + /// to memory containing an `Account` struct, immediately followed by + /// the account's data region. + #[inline(always)] + pub unsafe fn new_unchecked(raw: *mut Account) -> Self { + Self { raw } + } + /// Address of the account. #[inline(always)] pub const fn key(&self) -> &Address { From dfa004e279d3d358b762b788c5a6805d0ba304b9 Mon Sep 17 00:00:00 2001 From: febo Date: Wed, 24 Sep 2025 22:37:15 -0300 Subject: [PATCH 42/51] Add account_ptr method --- account-view/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index ca7819260..71cee4b3e 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -458,6 +458,11 @@ impl AccountView { write_bytes(self.data_ptr().sub(48), 0, 48); } + /// Returns the raw pointer to the `Account` struct. + pub const fn account_ptr(&self) -> *const Account { + self.raw + } + /// Returns the memory address of the account data. /// /// # Important From 3f2329f0a7899d0ce548344aa71f29f5cec77698 Mon Sep 17 00:00:00 2001 From: febo Date: Fri, 26 Sep 2025 07:55:37 -0300 Subject: [PATCH 43/51] Add copy feature --- account-view/Cargo.toml | 5 ++++- account-view/src/lib.rs | 14 ++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/account-view/Cargo.toml b/account-view/Cargo.toml index 691695994..d5d9eb8dd 100644 --- a/account-view/Cargo.toml +++ b/account-view/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "solana-account-view" -description = "A lightweight representation of a runtime account" +description = "Lightweight representation of a runtime account" documentation = "https://docs.rs/solana-account-view" version = "0.0.0" rust-version = "1.81.0" @@ -10,6 +10,9 @@ homepage = { workspace = true } license = { workspace = true } edition = { workspace = true } +[features] +copy = ["solana-address/copy"] + [dependencies] solana-address = { workspace = true } solana-program-error = { workspace = true } diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 71cee4b3e..dae0e9bc0 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -19,6 +19,8 @@ use { pub const MAX_PERMITTED_DATA_INCREASE: usize = 1_024 * 10; /// Value to indicate that an account is not borrowed. +/// +/// This value is the same as `solana_program_entrypoint::NON_DUP_MARKER`. pub const NOT_BORROWED: u8 = u8::MAX; /// Raw account data. @@ -28,16 +30,19 @@ pub const NOT_BORROWED: u8 = u8::MAX; /// directly after the `Account` struct in memory, with its size specified /// by [`Account::data_len`]. #[repr(C)] -#[derive(Clone, Copy, Default)] +#[cfg_attr(feature = "copy", derive(Copy))] +#[derive(Clone, Default)] pub struct Account { - /// Borrow state for lamports and account data. + /// Borrow state for account data. /// /// This reuses the memory reserved for the duplicate flag in the /// account to track data borrows. It represents the numbers of /// borrows available. The value `0` indicates that the account /// data is mutably borrowed, while values between `2` and `255` /// indicate the number of immutable borrows that can still be - /// allocated. + /// allocated. An account's data can only be mutably borrowed + /// when there are no other active borrows, i.e., when this value + /// is equal to [`NOT_BORROWED`]. pub borrow_state: u8, /// Indicates whether the transaction was signed by this account. @@ -87,7 +92,8 @@ pub struct Account { /// These conditions must always hold for any `AccountView` created from /// a raw pointer. #[repr(C)] -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "copy", derive(Copy))] +#[derive(Clone, PartialEq, Eq, Debug)] pub struct AccountView { /// Raw (pointer to) account data. /// From 92df2e4879b69253c133a4b5a16c24020ae84f1d Mon Sep 17 00:00:00 2001 From: febo Date: Thu, 2 Oct 2025 01:33:38 +0100 Subject: [PATCH 44/51] Add docs configuration --- account-view/Cargo.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/account-view/Cargo.toml b/account-view/Cargo.toml index d5d9eb8dd..431293d2b 100644 --- a/account-view/Cargo.toml +++ b/account-view/Cargo.toml @@ -10,6 +10,11 @@ homepage = { workspace = true } license = { workspace = true } edition = { workspace = true } +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] +all-features = true +rustdoc-args = ["--cfg=docsrs"] + [features] copy = ["solana-address/copy"] From 5f3065992afa77fbcfdbc3aa73d507b1d60a6258 Mon Sep 17 00:00:00 2001 From: febo Date: Thu, 2 Oct 2025 01:33:58 +0100 Subject: [PATCH 45/51] Rename to address --- account-view/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index dae0e9bc0..9a8d69665 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -62,8 +62,8 @@ pub struct Account { /// value is zero at the start of the instruction. pub resize_delta: i32, - /// Public key of the account. - pub key: Address, + /// Address of the account. + pub address: Address, /// Program that owns this account. Modifiable by programs. pub owner: Address, @@ -116,9 +116,9 @@ impl AccountView { /// Address of the account. #[inline(always)] - pub const fn key(&self) -> &Address { + pub const fn address(&self) -> &Address { // SAFETY: The `raw` pointer is guaranteed to be valid. - unsafe { &(*self.raw).key } + unsafe { &(*self.raw).address } } /// Return a reference to the address of the program that owns this account. From 59745aba3e43f3c8be02cdc2cf6178c748220be5 Mon Sep 17 00:00:00 2001 From: febo Date: Thu, 2 Oct 2025 02:36:33 +0100 Subject: [PATCH 46/51] Use clone --- account-view/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 9a8d69665..5c5833dd6 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -215,9 +215,10 @@ impl AccountView { /// /// It is undefined behavior to use this method while there is an active reference /// to the `owner` returned by [`Self::owner`]. + #[allow(clippy::clone_on_copy)] #[inline(always)] pub unsafe fn assign(&self, new_owner: &Address) { - write(&mut (*self.raw).owner, *new_owner); + write(&mut (*self.raw).owner, new_owner.clone()); } /// Return `true` if the account data is borrowed in any form. From 6903d5eef68d567f446f84678d2e1a7fe7e48d39 Mon Sep 17 00:00:00 2001 From: febo Date: Fri, 3 Oct 2025 12:25:58 +0100 Subject: [PATCH 47/51] Remove unused --- Cargo.lock | 7 --- address/Cargo.toml | 2 - address/src/derive.rs | 142 ------------------------------------------ address/src/lib.rs | 2 - 4 files changed, 153 deletions(-) delete mode 100644 address/src/derive.rs diff --git a/Cargo.lock b/Cargo.lock index 3ccf64bef..7fc860348 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2632,12 +2632,6 @@ dependencies = [ "digest", ] -[[package]] -name = "sha2-const-stable" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f179d4e11094a893b82fff208f74d448a7512f99f5a0acbd5c679b705f83ed9" - [[package]] name = "sha3" version = "0.10.8" @@ -2779,7 +2773,6 @@ dependencies = [ "rand", "serde", "serde_derive", - "sha2-const-stable", "solana-account-info", "solana-address", "solana-atomic-u64", diff --git a/address/Cargo.toml b/address/Cargo.toml index 9bbee4c14..effb77807 100644 --- a/address/Cargo.toml +++ b/address/Cargo.toml @@ -22,7 +22,6 @@ bytemuck = ["copy", "dep:bytemuck", "dep:bytemuck_derive"] copy = [] curve25519 = ["dep:curve25519-dalek", "error", "sha2"] decode = ["dep:five8", "dep:five8_const", "error"] -derive = ["dep:sha2-const-stable", "dep:solana-define-syscall"] dev-context-only-utils = ["dep:arbitrary", "rand"] error = ["dep:solana-program-error"] frozen-abi = [ @@ -49,7 +48,6 @@ five8_const = { workspace = true, optional = true } rand = { workspace = true, optional = true } serde = { workspace = true, optional = true } serde_derive = { workspace = true, optional = true } -sha2-const-stable = { version = "0.1.0", optional = true } solana-atomic-u64 = { workspace = true, optional = true } solana-frozen-abi = { workspace = true, features = ["frozen-abi"], optional = true } solana-frozen-abi-macro = { workspace = true, features = ["frozen-abi"], optional = true } diff --git a/address/src/derive.rs b/address/src/derive.rs deleted file mode 100644 index c8c869bdf..000000000 --- a/address/src/derive.rs +++ /dev/null @@ -1,142 +0,0 @@ -use crate::{Address, MAX_SEEDS, PDA_MARKER}; -#[cfg(target_os = "solana")] -use {core::mem::MaybeUninit, solana_define_syscall::definitions::sol_sha256}; - -/// Derive a [program address][pda] from the given seeds, optional bump and -/// program id. -/// -/// [pda]: https://solana.com/docs/core/pda -/// -/// In general, the derivation uses an optional bump (byte) value to ensure a -/// valid PDA (off-curve) is generated. Even when a program stores a bump to -/// derive a program address, it is necessary to use the -/// `Address::create_program_address` to validate the derivation. In -/// most cases, the program has the correct seeds for the derivation, so it would -/// be sufficient to just perform the derivation and compare it against the -/// expected resulting address. -/// -/// This function avoids the cost of the `create_program_address` syscall -/// (`1500` compute units) by directly computing the derived address -/// calculating the hash of the seeds, bump and program id using the -/// `sol_sha256` syscall. -/// -/// # Important -/// -/// This function differs from `Address::create_program_address` in that -/// it does not perform a validation to ensure that the derived address is a valid -/// (off-curve) program derived address. It is intended for use in cases where the -/// seeds, bump, and program id are known to be valid, and the caller wants to derive -/// the address without incurring the cost of the `create_program_address` syscall. -pub fn derive_address( - seeds: &[&[u8]; N], - bump: Option, - program_id: &Address, -) -> Address { - #[cfg(target_os = "solana")] - { - const { - assert!(N < MAX_SEEDS, "number of seeds must be less than MAX_SEEDS"); - } - - const UNINIT: MaybeUninit<&[u8]> = MaybeUninit::<&[u8]>::uninit(); - let mut data = [UNINIT; MAX_SEEDS + 2]; - let mut i = 0; - - while i < N { - // SAFETY: `data` is guaranteed to have enough space for `N` seeds, - // so `i` will always be within bounds. - unsafe { - data.get_unchecked_mut(i).write(seeds.get_unchecked(i)); - } - i += 1; - } - - // TODO: replace this with `as_slice` when the MSRV is upgraded - // to `1.84.0+`. - let bump_seed = [bump.unwrap_or_default()]; - - // SAFETY: `data` is guaranteed to have enough space for `MAX_SEEDS + 2` - // elements, and `MAX_SEEDS` is as large as `N`. - unsafe { - if bump.is_some() { - data.get_unchecked_mut(i).write(&bump_seed); - i += 1; - } - data.get_unchecked_mut(i).write(program_id.as_ref()); - data.get_unchecked_mut(i + 1).write(PDA_MARKER.as_ref()); - } - - let mut pda = MaybeUninit::
::uninit(); - - // SAFETY: `data` has `i + 2` elements initialized. - unsafe { - sol_sha256( - data.as_ptr() as *const u8, - (i + 2) as u64, - pda.as_mut_ptr() as *mut u8, - ); - } - - // SAFETY: `pda` has been initialized by the syscall. - unsafe { pda.assume_init() } - } - - #[cfg(not(target_os = "solana"))] - derive_address_const(seeds, bump, program_id) -} - -/// Derive a [program address][pda] from the given seeds, optional bump and -/// program id. -/// -/// [pda]: https://solana.com/docs/core/pda -/// -/// In general, the derivation uses an optional bump (byte) value to ensure a -/// valid PDA (off-curve) is generated. -/// -/// This function is intended for use in `const` contexts - i.e., the seeds and -/// bump are known at compile time and the program id is also a constant. It avoids -/// the cost of the `create_program_address` syscall (`1500` compute units) by -/// directly computing the derived address using the SHA-256 hash of the seeds, -/// bump and program id. -/// -/// # Important -/// -/// This function differs from `Address::create_program_address` in that -/// it does not perform a validation to ensure that the derived address is a valid -/// (off-curve) program derived address. It is intended for use in cases where the -/// seeds, bump, and program id are known to be valid, and the caller wants to derive -/// the address without incurring the cost of the `create_program_address` syscall. -/// -/// This function is a compile-time constant version of [`derive_address`]. -pub const fn derive_address_const( - seeds: &[&[u8]; N], - bump: Option, - program_id: &Address, -) -> Address { - const { - assert!(N < MAX_SEEDS, "number of seeds must be less than MAX_SEEDS"); - } - - let mut hasher = sha2_const_stable::Sha256::new(); - let mut i = 0; - - while i < seeds.len() { - hasher = hasher.update(seeds[i]); - i += 1; - } - - // TODO: replace this with `bump.as_slice()` when the MSRV is - // upgraded to `1.84.0+`. - Address::new_from_array(if let Some(bump) = bump { - hasher - .update(&[bump]) - .update(program_id.as_array()) - .update(PDA_MARKER) - .finalize() - } else { - hasher - .update(program_id.as_array()) - .update(PDA_MARKER) - .finalize() - }) -} diff --git a/address/src/lib.rs b/address/src/lib.rs index 7120e3b5c..b7bbafbc4 100644 --- a/address/src/lib.rs +++ b/address/src/lib.rs @@ -8,8 +8,6 @@ #![cfg_attr(feature = "frozen-abi", feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] -#[cfg(feature = "derive")] -pub mod derive; #[cfg(feature = "error")] pub mod error; #[cfg(feature = "rand")] From 8df45c5fac60cbbd334ca938f616bd667f7aa738 Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 18 Oct 2025 01:07:46 +0100 Subject: [PATCH 48/51] Add missing doc_auto_cfg --- account-view/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 5c5833dd6..bae773788 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -1,6 +1,7 @@ //! Data structures to represent account information. #![no_std] +#![cfg_attr(docsrs, feature(doc_auto_cfg))] use { core::{ From c73db501b7f1ab5df72005f6d59df8ef41599fde Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 18 Oct 2025 01:33:08 +0100 Subject: [PATCH 49/51] Fix lints --- account-view/src/lib.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index bae773788..91b6142ed 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -2,6 +2,7 @@ #![no_std] #![cfg_attr(docsrs, feature(doc_auto_cfg))] +#![allow(clippy::arithmetic_side_effects)] use { core::{ @@ -117,7 +118,7 @@ impl AccountView { /// Address of the account. #[inline(always)] - pub const fn address(&self) -> &Address { + pub fn address(&self) -> &Address { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { &(*self.raw).address } } @@ -132,21 +133,21 @@ impl AccountView { /// which can be modified by `assign` and `close` methods. It is undefined /// behavior to use this reference after the account has been modified. #[inline(always)] - pub const unsafe fn owner(&self) -> &Address { + pub unsafe fn owner(&self) -> &Address { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { &(*self.raw).owner } } /// Indicate whether the transaction was signed by this account. #[inline(always)] - pub const fn is_signer(&self) -> bool { + pub fn is_signer(&self) -> bool { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).is_signer != 0 } } /// Indicate whether the account is writable. #[inline(always)] - pub const fn is_writable(&self) -> bool { + pub fn is_writable(&self) -> bool { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).is_writable != 0 } } @@ -155,14 +156,14 @@ impl AccountView { /// /// Program accounts are always read-only. #[inline(always)] - pub const fn executable(&self) -> bool { + pub fn executable(&self) -> bool { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).executable != 0 } } /// Return the size of the data in the account. #[inline(always)] - pub const fn data_len(&self) -> usize { + pub fn data_len(&self) -> usize { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).data_len as usize } } @@ -173,21 +174,21 @@ impl AccountView { /// This value will be different than zero if the account has been /// resized during the current instruction. #[inline(always)] - pub const fn resize_delta(&self) -> i32 { + pub fn resize_delta(&self) -> i32 { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).resize_delta } } /// Return the lamports in the account. #[inline(always)] - pub const fn lamports(&self) -> u64 { + pub fn lamports(&self) -> u64 { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).lamports } } /// Set the lamports in the account. #[inline(always)] - pub const fn set_lamports(&self, lamports: u64) { + pub fn set_lamports(&self, lamports: u64) { // SAFETY: The `raw` pointer is guaranteed to be valid. unsafe { (*self.raw).lamports = lamports; @@ -198,7 +199,7 @@ impl AccountView { /// /// An account is considered empty if the data length is zero. #[inline(always)] - pub const fn is_data_empty(&self) -> bool { + pub fn is_data_empty(&self) -> bool { // SAFETY: The `raw` pointer is guaranteed to be valid. self.data_len() == 0 } @@ -259,7 +260,7 @@ impl AccountView { /// Tries to get a read-only reference to the data field, failing if the field /// is already mutable borrowed or if `7` borrows already exist. - pub fn try_borrow_data(&self) -> Result, ProgramError> { + pub fn try_borrow_data(&self) -> Result, ProgramError> { // check if the account data is already borrowed self.can_borrow_data()?; @@ -282,7 +283,7 @@ impl AccountView { /// Tries to get a mutable reference to the data field, failing if the field /// is already borrowed in any form. - pub fn try_borrow_data_mut(&self) -> Result, ProgramError> { + pub fn try_borrow_data_mut(&self) -> Result, ProgramError> { // check if the account data is already borrowed self.can_borrow_data_mut()?; @@ -441,10 +442,9 @@ impl AccountView { /// The lamports must be moved from the account prior to closing it to prevent /// an unbalanced instruction error. /// - /// If [`Self::realloc`] or [`Self::resize`] are called after closing the account, - /// they might incorrectly return an error for going over the limit if the account - /// previously had space allocated since this method does not update the - /// [`Self::resize_delta`] value. + /// If [`Self::resize`] is called after closing the account, it might incorrectly + /// return an error for going over the limit if the account previously had space + /// allocated since this method does not update the [`Self::resize_delta`] value. /// /// # Safety /// From 8018528ddc01e3d04f3b5baaea67c1c22deebb8b Mon Sep 17 00:00:00 2001 From: febo Date: Sat, 18 Oct 2025 01:46:53 +0100 Subject: [PATCH 50/51] Add no_std check --- scripts/check-no-std.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/check-no-std.sh b/scripts/check-no-std.sh index b04511276..f9a99d071 100755 --- a/scripts/check-no-std.sh +++ b/scripts/check-no-std.sh @@ -9,6 +9,7 @@ cd "${src_root}" no_std_crates=( -p solana-address + -p solana-account-view -p solana-clock -p solana-commitment-config -p solana-define-syscall From e7ba5b2a922a40bc2fe4a904c125e512c0e7e726 Mon Sep 17 00:00:00 2001 From: febo Date: Wed, 22 Oct 2025 23:58:01 +0100 Subject: [PATCH 51/51] Rename account type --- account-view/src/lib.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/account-view/src/lib.rs b/account-view/src/lib.rs index 91b6142ed..4048bd9e8 100644 --- a/account-view/src/lib.rs +++ b/account-view/src/lib.rs @@ -30,11 +30,11 @@ pub const NOT_BORROWED: u8 = u8::MAX; /// This struct is wrapped by [`AccountView`], which provides safe access /// to account information. At runtime, the account's data is serialized /// directly after the `Account` struct in memory, with its size specified -/// by [`Account::data_len`]. +/// by [`RuntimeAccount::data_len`]. #[repr(C)] #[cfg_attr(feature = "copy", derive(Copy))] #[derive(Clone, Default)] -pub struct Account { +pub struct RuntimeAccount { /// Borrow state for account data. /// /// This reuses the memory reserved for the duplicate flag in the @@ -100,7 +100,7 @@ pub struct AccountView { /// Raw (pointer to) account data. /// /// Note that this is a pointer can be shared across multiple `AccountView`. - raw: *mut Account, + raw: *mut RuntimeAccount, } impl AccountView { @@ -112,7 +112,7 @@ impl AccountView { /// to memory containing an `Account` struct, immediately followed by /// the account's data region. #[inline(always)] - pub unsafe fn new_unchecked(raw: *mut Account) -> Self { + pub unsafe fn new_unchecked(raw: *mut RuntimeAccount) -> Self { Self { raw } } @@ -467,7 +467,7 @@ impl AccountView { } /// Returns the raw pointer to the `Account` struct. - pub const fn account_ptr(&self) -> *const Account { + pub const fn account_ptr(&self) -> *const RuntimeAccount { self.raw } @@ -483,7 +483,7 @@ impl AccountView { #[inline(always)] pub fn data_ptr(&self) -> *mut u8 { // SAFETY: The `raw` pointer is guaranteed to be valid. - unsafe { (self.raw as *mut u8).add(size_of::()) } + unsafe { (self.raw as *mut u8).add(size_of::()) } } } @@ -735,8 +735,8 @@ mod tests { #[test] fn test_borrow_data() { // 8-bytes aligned account data + 8 bytes of trailing data. - let mut data = [0u64; size_of::() / size_of::() + 1]; - let account = data.as_mut_ptr() as *mut Account; + let mut data = [0u64; size_of::() / size_of::() + 1]; + let account = data.as_mut_ptr() as *mut RuntimeAccount; unsafe { (*account).data_len = 8 }; data[0] = NOT_BORROWED as u64; @@ -819,7 +819,7 @@ mod tests { data[10] = 100; let account = AccountView { - raw: data.as_mut_ptr() as *const _ as *mut Account, + raw: data.as_mut_ptr() as *const _ as *mut RuntimeAccount, }; assert_eq!(account.data_len(), 100);