瀏覽代碼

Remove input offset counter (#153)

* Remove offset counter

* Fix doc

* Update readme

* Fix review comments

* Revert offset increment change

* More review comments
Fernando Otero 6 月之前
父節點
當前提交
1720bde000
共有 4 個文件被更改,包括 91 次插入96 次删除
  1. 2 2
      README.md
  2. 49 54
      sdk/pinocchio/src/entrypoint/lazy.rs
  3. 38 38
      sdk/pinocchio/src/entrypoint/mod.rs
  4. 2 2
      sdk/pinocchio/src/lib.rs

+ 2 - 2
README.md

@@ -132,8 +132,8 @@ pub fn process_instruction(
 
 The `InstructionContext` provides on-demand access to the information of the input:
 
-* `available()`: number of available accounts
-* `next_account()`: parsers the next available account (can be used as many times as accounts available)
+* `remaining()`: number of remaining accounts to be parsed
+* `next_account()`: parsers the next available account (can be used as many times as accounts remaining)
 * `instruction_data()`: parsers the instruction data
 * `program_id()`: parsers the program id
 

+ 49 - 54
sdk/pinocchio/src/entrypoint/lazy.rs

@@ -2,7 +2,8 @@
 //! input buffer.
 
 use crate::{
-    account_info::{Account, AccountInfo, MAX_PERMITTED_DATA_INCREASE},
+    account_info::{Account, AccountInfo},
+    entrypoint::STATIC_ACCOUNT_DATA,
     program_error::ProgramError,
     pubkey::Pubkey,
     BPF_ALIGN_OF_U128, NON_DUP_MARKER,
@@ -100,16 +101,15 @@ macro_rules! lazy_program_entrypoint {
 /// This is a wrapper around the input buffer that provides methods to read the accounts
 /// and instruction data. It is used by the lazy entrypoint to access the input data on demand.
 pub struct InstructionContext {
-    /// Pointer to the runtime input buffer for the instruction.
-    input: *mut u8,
+    /// Pointer to the runtime input buffer to read from.
+    ///
+    /// This pointer is moved forward as accounts are read from the buffer.
+    buffer: *mut u8,
 
     /// Number of remaining accounts.
     ///
     /// This value is decremented each time [`next_account`] is called.
     remaining: u64,
-
-    /// Current memory offset on the input buffer.
-    offset: usize,
 }
 
 impl InstructionContext {
@@ -136,11 +136,13 @@ impl InstructionContext {
     #[inline(always)]
     pub unsafe fn new_unchecked(input: *mut u8) -> Self {
         Self {
-            input,
             // SAFETY: The first 8 bytes of the input buffer represent the
-            // number of accounts when serialized by the SVM loader.
+            // number of accounts when serialized by the SVM loader, which is read
+            // when the context is created.
+            buffer: unsafe { input.add(core::mem::size_of::<u64>()) },
+            // SAFETY: Read the number of accounts from the input buffer serialized
+            // by the SVM loader.
             remaining: unsafe { *(input as *const u64) },
-            offset: core::mem::size_of::<u64>(),
         }
     }
 
@@ -161,13 +163,13 @@ impl InstructionContext {
             .checked_sub(1)
             .ok_or(ProgramError::NotEnoughAccountKeys)?;
 
-        Ok(unsafe { read_account(self.input, &mut self.offset) })
+        Ok(unsafe { self.read_account() })
     }
 
     /// Returns the next account for the instruction.
     ///
     /// Note that this method does *not* decrement the number of remaining accounts, but moves
-    /// the offset forward. It is intended for use when the caller is certain on the number of
+    /// the input pointer forward. It is intended for use when the caller is certain on the number of
     /// remaining accounts.
     ///
     /// # Safety
@@ -176,13 +178,7 @@ impl InstructionContext {
     /// there are no more remaining accounts results in undefined behavior.
     #[inline(always)]
     pub unsafe fn next_account_unchecked(&mut self) -> MaybeAccount {
-        read_account(self.input, &mut self.offset)
-    }
-
-    /// Returns the number of available accounts.
-    #[inline(always)]
-    pub fn available(&self) -> u64 {
-        unsafe { *(self.input as *const u64) }
+        self.read_account()
     }
 
     /// Returns the number of remaining accounts.
@@ -193,7 +189,7 @@ impl InstructionContext {
         self.remaining
     }
 
-    /// Returns the instruction data for the instruction.
+    /// Returns the data for the instruction.
     ///
     /// This method can only be used after all accounts have been read; otherwise, it will
     /// return a [`ProgramError::InvalidInstructionData`] error.
@@ -214,10 +210,10 @@ impl InstructionContext {
     /// before reading all accounts will result in undefined behavior.
     #[inline(always)]
     pub unsafe fn instruction_data_unchecked(&self) -> &[u8] {
-        let data_len = *(self.input.add(self.offset) as *const usize);
-        // shadowing the offset to avoid leaving it in an inconsistent state
-        let offset = self.offset + core::mem::size_of::<u64>();
-        core::slice::from_raw_parts(self.input.add(offset), data_len)
+        let data_len = *(self.buffer as *const usize);
+        // shadowing the input to avoid leaving it in an inconsistent position
+        let data = self.buffer.add(core::mem::size_of::<u64>());
+        core::slice::from_raw_parts(data, data_len)
     }
 
     /// Returns the program id for the instruction.
@@ -241,10 +237,36 @@ impl InstructionContext {
     /// before reading all accounts will result in undefined behavior.
     #[inline(always)]
     pub unsafe fn program_id_unchecked(&self) -> &Pubkey {
-        let data_len = *(self.input.add(self.offset) as *const usize);
-        &*(self
-            .input
-            .add(self.offset + core::mem::size_of::<u64>() + data_len) as *const Pubkey)
+        let data_len = *(self.buffer as *const usize);
+        &*(self.buffer.add(core::mem::size_of::<u64>() + data_len) as *const Pubkey)
+    }
+
+    /// Read an account from the input buffer.
+    ///
+    /// This can only be called with a buffer that was serialized by the runtime as
+    /// it assumes a specific memory layout.
+    #[allow(clippy::cast_ptr_alignment, clippy::missing_safety_doc)]
+    #[inline(always)]
+    unsafe fn read_account(&mut self) -> MaybeAccount {
+        let account: *mut Account = self.buffer as *mut Account;
+        // Adds an 8-bytes offset for:
+        //   - rent epoch in case of a non-duplicate account
+        //   - duplicate marker + 7 bytes of padding in case of a duplicate account
+        self.buffer = self.buffer.add(core::mem::size_of::<u64>());
+
+        if (*account).borrow_state == NON_DUP_MARKER {
+            // Unique account: repurpose the borrow state to track borrows.
+            (*account).borrow_state = 0b_0000_0000;
+
+            self.buffer = self.buffer.add(STATIC_ACCOUNT_DATA);
+            self.buffer = self.buffer.add((*account).data_len as usize);
+            self.buffer = self.buffer.add(self.buffer.align_offset(BPF_ALIGN_OF_U128));
+
+            MaybeAccount::Account(AccountInfo { raw: account })
+        } else {
+            // The caller will handle the mapping to the original account.
+            MaybeAccount::Duplicated((*account).borrow_state)
+        }
     }
 }
 
@@ -271,30 +293,3 @@ impl MaybeAccount {
         account
     }
 }
-
-/// Read an account from the input buffer.
-///
-/// This can only be called with a buffer that was serialized by the runtime as
-/// it assumes a specific memory layout.
-#[allow(clippy::cast_ptr_alignment, clippy::missing_safety_doc)]
-#[inline(always)]
-unsafe fn read_account(input: *mut u8, offset: &mut usize) -> MaybeAccount {
-    let account: *mut Account = input.add(*offset) as *mut _;
-
-    if (*account).borrow_state == NON_DUP_MARKER {
-        // repurpose the borrow state to track borrows
-        (*account).borrow_state = 0b_0000_0000;
-
-        *offset += core::mem::size_of::<Account>();
-        *offset += (*account).data_len as usize;
-        *offset += MAX_PERMITTED_DATA_INCREASE;
-        *offset += (*offset as *const u8).align_offset(BPF_ALIGN_OF_U128);
-        *offset += core::mem::size_of::<u64>();
-
-        MaybeAccount::Account(AccountInfo { raw: account })
-    } else {
-        *offset += core::mem::size_of::<u64>();
-        //the caller will handle the mapping to the original account
-        MaybeAccount::Duplicated((*account).borrow_state)
-    }
-}

+ 38 - 38
sdk/pinocchio/src/entrypoint/mod.rs

@@ -30,6 +30,11 @@ pub type ProgramResult = super::ProgramResult;
 /// Return value for a successful program execution.
 pub const SUCCESS: u64 = super::SUCCESS;
 
+/// The "static" size of an account in the input buffer.
+///
+/// This is the size of the account header plus the maximum permitted data increase.
+const STATIC_ACCOUNT_DATA: usize = core::mem::size_of::<Account>() + MAX_PERMITTED_DATA_INCREASE;
+
 /// Declare the program entrypoint and set up global handlers.
 ///
 /// The main difference from the standard (SDK) [`entrypoint`](https://docs.rs/solana-program-entrypoint/latest/solana_program_entrypoint/macro.entrypoint.html)
@@ -160,37 +165,37 @@ macro_rules! program_entrypoint {
 #[allow(clippy::cast_ptr_alignment, clippy::missing_safety_doc)]
 #[inline(always)]
 pub unsafe fn deserialize<'a, const MAX_ACCOUNTS: usize>(
-    input: *mut u8,
+    mut input: *mut u8,
     accounts: &mut [core::mem::MaybeUninit<AccountInfo>],
 ) -> (&'a Pubkey, usize, &'a [u8]) {
-    let mut offset: usize = 0;
-
     // total number of accounts present; it only process up to MAX_ACCOUNTS
-    let total_accounts = *(input.add(offset) as *const u64) as usize;
-    offset += core::mem::size_of::<u64>();
+    let mut processed = *(input as *const u64) as usize;
+    input = input.add(core::mem::size_of::<u64>());
 
-    let processed = if total_accounts > 0 {
+    if processed > 0 {
+        let total_accounts = processed;
         // number of accounts to process (limited to MAX_ACCOUNTS)
-        let processed = core::cmp::min(total_accounts, MAX_ACCOUNTS);
+        processed = core::cmp::min(total_accounts, MAX_ACCOUNTS);
 
         for i in 0..processed {
-            let account_info: *mut Account = input.add(offset) as *mut _;
+            let account_info: *mut Account = input as *mut Account;
+            // Adds an 8-bytes offset for:
+            //   - rent epoch in case of a non-duplicated account
+            //   - duplicated marker + 7 bytes of padding in case of a duplicated account
+            input = input.add(core::mem::size_of::<u64>());
 
             if (*account_info).borrow_state == NON_DUP_MARKER {
-                // repurpose the borrow state to track borrows
+                // Unique account: repurpose the borrow state to track borrows.
                 (*account_info).borrow_state = 0b_0000_0000;
 
-                offset += core::mem::size_of::<Account>();
-                offset += (*account_info).data_len as usize;
-                offset += MAX_PERMITTED_DATA_INCREASE;
-                offset += (offset as *const u8).align_offset(BPF_ALIGN_OF_U128);
-                offset += core::mem::size_of::<u64>();
+                input = input.add(STATIC_ACCOUNT_DATA);
+                input = input.add((*account_info).data_len as usize);
+                input = input.add(input.align_offset(BPF_ALIGN_OF_U128));
 
                 accounts[i].write(AccountInfo { raw: account_info });
             } else {
-                offset += core::mem::size_of::<u64>();
-                // duplicated account – clone the original pointer using `borrow_state` since it represents the
-                // index of the duplicated account passed by the runtime.
+                // Duplicated account: clone the original pointer using `borrow_state` since it represents
+                // the index of the duplicated account passed by the runtime.
                 accounts[i].write(
                     accounts
                         .get_unchecked((*account_info).borrow_state as usize)
@@ -200,38 +205,33 @@ pub unsafe fn deserialize<'a, const MAX_ACCOUNTS: usize>(
             }
         }
 
-        // process any remaining accounts to move the offset to the instruction
+        // Process any remaining accounts to move the offset to the instruction
         // data (there is a duplication of logic but we avoid testing whether we
-        // have space for the account or not)
+        // have space for the account or not).
         for _ in processed..total_accounts {
-            let account_info: *mut Account = input.add(offset) as *mut _;
+            let account_info: *mut Account = input as *mut Account;
+            // Adds an 8-bytes offset for:
+            //   - rent epoch in case of a non-duplicate account
+            //   - duplicate marker + 7 bytes of padding in case of a duplicate account
+            input = input.add(core::mem::size_of::<u64>());
 
             if (*account_info).borrow_state == NON_DUP_MARKER {
-                offset += core::mem::size_of::<Account>();
-                offset += (*account_info).data_len as usize;
-                offset += MAX_PERMITTED_DATA_INCREASE;
-                offset += (offset as *const u8).align_offset(BPF_ALIGN_OF_U128);
-                offset += core::mem::size_of::<u64>();
-            } else {
-                offset += core::mem::size_of::<u64>();
+                input = input.add(STATIC_ACCOUNT_DATA);
+                input = input.add((*account_info).data_len as usize);
+                input = input.add(input.align_offset(BPF_ALIGN_OF_U128));
             }
         }
-
-        processed
-    } else {
-        // no accounts to process
-        0
-    };
+    }
 
     // instruction data
-    let instruction_data_len = *(input.add(offset) as *const u64) as usize;
-    offset += core::mem::size_of::<u64>();
+    let instruction_data_len = *(input as *const u64) as usize;
+    input = input.add(core::mem::size_of::<u64>());
 
-    let instruction_data = { core::slice::from_raw_parts(input.add(offset), instruction_data_len) };
-    offset += instruction_data_len;
+    let instruction_data = { core::slice::from_raw_parts(input, instruction_data_len) };
+    input = input.add(instruction_data_len);
 
     // program id
-    let program_id: &Pubkey = &*(input.add(offset) as *const Pubkey);
+    let program_id: &Pubkey = &*(input as *const Pubkey);
 
     (program_id, processed, instruction_data)
 }

+ 2 - 2
sdk/pinocchio/src/lib.rs

@@ -113,8 +113,8 @@
 //! The [`InstructionContext`](entrypoint::InstructionContext) provides on-demand
 //! access to the information of the input:
 //!
-//! * [`available()`](entrypoint::InstructionContext::available): number of available
-//!   accounts.
+//! * [`remaining()`](entrypoint::InstructionContext::remaining): number of available
+//!   accounts to parse; this number is decremented as the program parses accounts.
 //! * [`next_account()`](entrypoint::InstructionContext::next_account): parses the
 //!   next available account (can be used as many times as accounts available).
 //! * [`instruction_data()`](entrypoint::InstructionContext::instruction_data): parses