Skip to content

Clarify the use of constant values #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 16, 2025
Merged

Clarify the use of constant values #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c88b078
Add comments on constants
febo Jul 15, 2025
9b7eb63
Improve offset comments
febo Jul 15, 2025
39b60b1
Add bitmask to dictionary
febo Jul 16, 2025
ecc42e9
Renamed to field_at_offset
febo Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion scripts/setup/solana.dic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ callee
RPC
ed25519
performant
syscall/S
syscall/S
bitmask
Copy link
Contributor

@ilya-bobyr ilya-bobyr Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor

Seems like this file does not have a new line at the end.
Here is one set of arguments as to why this might matter: https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline

121 changes: 69 additions & 52 deletions sdk/pinocchio/src/account_info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
})
}
Expand All @@ -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,
})
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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,
})
}
Expand All @@ -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,
})
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -682,18 +699,18 @@ impl<T: ?Sized> 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<T>,
state: NonNull<u8>,
/// 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>,
Expand All @@ -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,
}
}
Expand All @@ -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,
})
}
Expand All @@ -756,7 +773,7 @@ impl<T: ?Sized> core::ops::DerefMut for RefMut<'_, T> {
impl<T: ?Sized> 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 };
}
}

Expand All @@ -771,31 +788,31 @@ 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,
};

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);
Expand All @@ -806,31 +823,31 @@ 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,
};

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);
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 21 additions & 5 deletions sdk/pinocchio/src/instruction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,35 @@ pub struct Account<'a> {
_account_info: PhantomData<&'a AccountInfo>,
}

/// Return a pointer to a type `U` given type `T` has a field of type `U` at the specified
/// offset (in bytes) from the start of the `T` type.
///
/// # Safety
///
/// The caller must ensure that the `ptr` is a valid pointer to a type `T`, and `ptr + offset`
/// points to bytes that are properly aligned for `U` and represent a bit pattern that is a
/// valid instance of `U`.
///
/// If any of this requirements is not valid, this function leads to undefined behavior.
#[inline(always)]
const fn offset<T, U>(ptr: *const T, offset: usize) -> *const U {
const unsafe fn field_at_offset<T, U>(ptr: *const T, offset: usize) -> *const U {
// SAFETY: The caller ensures that the offset is valid for the type `T` and that
// the resulting pointer is valid for type `U`.
unsafe { (ptr as *const u8).add(offset) as *const U }
}

impl<'a> From<&'a AccountInfo> for Account<'a> {
fn from(account: &'a AccountInfo) -> Self {
Account {
key: offset(account.raw, 8),
lamports: offset(account.raw, 72),
// SAFETY: offset `8` is the `key` field in the `Account` struct.
key: unsafe { field_at_offset(account.raw, 8) },
// SAFETY: offset `72` is the `lamports` field in the `Account` struct.
lamports: unsafe { field_at_offset(account.raw, 72) },
data_len: account.data_len() as u64,
data: offset(account.raw, 88),
owner: offset(account.raw, 40),
// SAFETY: offset `88` is the start of the account data in the `Account` struct.
data: unsafe { field_at_offset(account.raw, 88) },
// SAFETY: offset `40` is the `owner` field in the `Account` struct.
owner: unsafe { field_at_offset(account.raw, 40) },
// The `rent_epoch` field is not present in the `AccountInfo` struct,
// since the value occurs after the variable data of the account in
// the runtime input data.
Expand Down