Simplify transaction context (#7537)

* Remove try_borrow_program_account

* Simplify borrowed account

* Remove unused argument

* Switch from recell to cell

* Simplify borrowed account

* Remove unnecessary derives

* Rename field

* Fix rebase conflicts

Cargo.lock

@@ -11384,6 +11384,7 @@ name = "solana-transaction-context"
 version = "3.0.0"
 dependencies = [
  "bincode",
+ "qualifier_attr",
  "serde",
  "serde_derive",
  "solana-account",

program-runtime/src/invoke_context.rs

@@ -366,7 +366,7 @@ impl<'a> InvokeContext<'a> {
                         instruction_account.set_is_writable(
                             instruction_account.is_writable() || account_meta.is_writable,
                         );
-                        instruction_account.clone()
+                        *instruction_account
                     };
                     instruction_accounts.push(cloned_account);
                 } else {
@@ -538,18 +538,15 @@ impl<'a> InvokeContext<'a> {
         let process_executable_chain_time = Measure::start("process_executable_chain_time");
 
         let builtin_id = {
-            let borrowed_root_account = instruction_context
-                .try_borrow_program_account(self.transaction_context)
-                .map_err(|_| InstructionError::UnsupportedProgramId)?;
-            let owner_id = borrowed_root_account.get_owner();
-            if native_loader::check_id(owner_id) {
-                *borrowed_root_account.get_key()
-            } else if bpf_loader_deprecated::check_id(owner_id)
-                || bpf_loader::check_id(owner_id)
-                || bpf_loader_upgradeable::check_id(owner_id)
-                || loader_v4::check_id(owner_id)
+            let owner_id = instruction_context.get_program_owner(self.transaction_context)?;
+            if native_loader::check_id(&owner_id) {
+                *instruction_context.get_program_key(self.transaction_context)?
+            } else if bpf_loader_deprecated::check_id(&owner_id)
+                || bpf_loader::check_id(&owner_id)
+                || bpf_loader_upgradeable::check_id(&owner_id)
+                || loader_v4::check_id(&owner_id)
             {
-                *owner_id
+                owner_id
             } else {
                 return Err(InstructionError::UnsupportedProgramId);
             }
@@ -709,12 +706,11 @@ impl<'a> InvokeContext<'a> {
         self.transaction_context
             .get_current_instruction_context()
             .and_then(|instruction_context| {
-                let program_account =
-                    instruction_context.try_borrow_program_account(self.transaction_context);
-                debug_assert!(program_account.is_ok());
-                program_account
+                let owner_id = instruction_context.get_program_owner(self.transaction_context);
+                debug_assert!(owner_id.is_ok());
+                owner_id
             })
-            .map(|program_account| *program_account.get_owner() != bpf_loader_deprecated::id())
+            .map(|owner_key| owner_key != bpf_loader_deprecated::id())
             .unwrap_or(true)
     }
 
@@ -1353,10 +1349,7 @@ mod tests {
 
         assert!(result.is_ok());
         assert_eq!(
-            invoke_context
-                .transaction_context
-                .accounts_resize_delta()
-                .unwrap(),
+            invoke_context.transaction_context.accounts_resize_delta(),
             resize_delta
         );
     }

program-runtime/src/serialization.rs

@@ -238,14 +238,9 @@ pub fn serialize_parameters(
         return Err(InstructionError::MaxAccountsExceeded);
     }
 
-    let (program_id, is_loader_deprecated) = {
-        let program_account =
-            instruction_context.try_borrow_program_account(transaction_context)?;
-        (
-            *program_account.get_key(),
-            *program_account.get_owner() == bpf_loader_deprecated::id(),
-        )
-    };
+    let program_id = *instruction_context.get_program_key(transaction_context)?;
+    let is_loader_deprecated =
+        instruction_context.get_program_owner(transaction_context)? == bpf_loader_deprecated::id();
 
     let accounts = (0..instruction_context.get_number_of_instruction_accounts())
         .map(|instruction_account_index| {
@@ -296,10 +291,8 @@ pub fn deserialize_parameters(
     buffer: &[u8],
     accounts_metadata: &[SerializedAccountMetadata],
 ) -> Result<(), InstructionError> {
-    let is_loader_deprecated = *instruction_context
-        .try_borrow_program_account(transaction_context)?
-        .get_owner()
-        == bpf_loader_deprecated::id();
+    let is_loader_deprecated =
+        instruction_context.get_program_owner(transaction_context)? == bpf_loader_deprecated::id();
     let account_lengths = accounts_metadata.iter().map(|a| a.original_data_len);
     if is_loader_deprecated {
         deserialize_parameters_unaligned(
@@ -1612,7 +1605,7 @@ mod tests {
                 .unwrap();
         }
         assert_eq!(
-            transaction_context.accounts_resize_delta().unwrap(),
+            transaction_context.accounts_resize_delta(),
             MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION
                 - remaining_allowed_growth as i64,
         );

programs/bpf_loader/src/lib.rs

@@ -381,11 +381,11 @@ pub(crate) fn process_instruction_inner(
     let log_collector = invoke_context.get_log_collector();
     let transaction_context = &invoke_context.transaction_context;
     let instruction_context = transaction_context.get_current_instruction_context()?;
-    let program_account = instruction_context.try_borrow_program_account(transaction_context)?;
+    let program_id = instruction_context.get_program_key(transaction_context)?;
+    let owner_id = instruction_context.get_program_owner(transaction_context)?;
 
     // Program Management Instruction
-    if native_loader::check_id(program_account.get_owner()) {
-        drop(program_account);
+    if native_loader::check_id(&owner_id) {
         let program_id = instruction_context.get_program_key(transaction_context)?;
         return if bpf_loader_upgradeable::check_id(program_id) {
             invoke_context.consume_checked(UPGRADEABLE_LOADER_COMPUTE_UNITS)?;
@@ -413,12 +413,11 @@ pub(crate) fn process_instruction_inner(
     let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time");
     let executor = invoke_context
         .program_cache_for_tx_batch
-        .find(program_account.get_key())
+        .find(program_id)
         .ok_or_else(|| {
             ic_logger_msg!(log_collector, "Program is not cached");
             InstructionError::UnsupportedProgramId
         })?;
-    drop(program_account);
     get_or_create_executor_time.stop();
     invoke_context.timings.get_or_create_executor_us += get_or_create_executor_time.as_us();
 
@@ -1524,14 +1523,9 @@ fn execute<'a, 'b: 'a>(
     let log_collector = invoke_context.get_log_collector();
     let transaction_context = &invoke_context.transaction_context;
     let instruction_context = transaction_context.get_current_instruction_context()?;
-    let (program_id, is_loader_deprecated) = {
-        let program_account =
-            instruction_context.try_borrow_program_account(transaction_context)?;
-        (
-            *program_account.get_key(),
-            *program_account.get_owner() == bpf_loader_deprecated::id(),
-        )
-    };
+    let program_id = *instruction_context.get_program_key(transaction_context)?;
+    let is_loader_deprecated =
+        instruction_context.get_program_owner(transaction_context)? == bpf_loader_deprecated::id();
     #[cfg(any(target_os = "windows", not(target_arch = "x86_64")))]
     let use_jit = false;
     #[cfg(all(not(target_os = "windows"), target_arch = "x86_64"))]

programs/sbf/Cargo.lock

@@ -9619,6 +9619,7 @@ name = "solana-transaction-context"
 version = "3.0.0"
 dependencies = [
  "bincode",
+ "qualifier_attr",
  "serde",
  "serde_derive",
  "solana-account",

runtime/src/bank/tests.rs

@@ -5203,9 +5203,10 @@ fn test_same_program_id_uses_unique_executable_accounts() {
     declare_process_instruction!(MockBuiltin, 1, |invoke_context| {
         let transaction_context = &invoke_context.transaction_context;
         let instruction_context = transaction_context.get_current_instruction_context()?;
-        instruction_context
-            .try_borrow_program_account(transaction_context)?
-            .set_data_length(2)
+        let program_idx = instruction_context.get_index_of_program_account_in_transaction()?;
+        let mut acc = transaction_context.accounts().try_borrow_mut(program_idx)?;
+        acc.set_data(vec![1, 2]);
+        Ok(())
     });
 
     let (genesis_config, mint_keypair) = create_genesis_config(50000);

svm/src/account_loader.rs

@@ -411,7 +411,6 @@ pub fn validate_fee_payer(
         &payer_pre_rent_state,
         &payer_post_rent_state,
         payer_address,
-        payer_account,
         payer_index,
     )
 }

svm/src/rent_calculator.rs

@@ -48,10 +48,6 @@ pub fn check_rent_state(
             transaction_context
                 .get_key_of_account_at_index(index)
                 .expect(expect_msg),
-            &transaction_context
-                .accounts()
-                .try_borrow(index)
-                .expect(expect_msg),
             index,
         )?;
     }
@@ -67,7 +63,6 @@ pub fn check_rent_state_with_account(
     pre_rent_state: &RentState,
     post_rent_state: &RentState,
     address: &Pubkey,
-    _account_state: &AccountSharedData,
     account_index: IndexOfAccount,
 ) -> TransactionResult<()> {
     if !solana_sdk_ids::incinerator::check_id(address)

transaction-context/Cargo.toml

@@ -16,10 +16,11 @@ rustdoc-args = ["--cfg=docsrs"]
 
 [features]
 bincode = ["dep:bincode", "serde", "solana-account/bincode"]
-dev-context-only-utils = ["bincode", "solana-account/dev-context-only-utils"]
+dev-context-only-utils = ["bincode", "solana-account/dev-context-only-utils", "dep:qualifier_attr"]
 serde = ["dep:serde", "dep:serde_derive"]
 
 [dependencies]
+qualifier_attr = { workspace = true, optional = true }
 serde = { workspace = true, optional = true }
 serde_derive = { workspace = true, optional = true }
 solana-account = { workspace = true }

transaction-context/src/lib.rs

@@ -2,6 +2,8 @@
 #![deny(clippy::indexing_slicing)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 
+#[cfg(feature = "dev-context-only-utils")]
+use qualifier_attr::qualifiers;
 #[cfg(not(target_os = "solana"))]
 use {solana_account::WritableAccount, solana_rent::Rent};
 use {
@@ -11,7 +13,7 @@ use {
     solana_pubkey::Pubkey,
     solana_sbpf::memory_region::{AccessType, AccessViolationHandler, MemoryRegion},
     std::{
-        cell::{Ref, RefCell, RefMut},
+        cell::{Cell, Ref, RefCell, RefMut},
         collections::HashSet,
         pin::Pin,
         rc::Rc,
@@ -61,7 +63,7 @@ pub type IndexOfAccount = u16;
 /// This data structure is supposed to be shared with programs in ABIv2, so do not modify it
 /// without consulting SIMD-0177.
 #[repr(C)]
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug)]
 pub struct InstructionAccount {
     /// Points to the account and its key in the `TransactionContext`
     pub index_in_transaction: IndexOfAccount,
@@ -104,12 +106,12 @@ impl InstructionAccount {
 /// An account key and the matching account
 pub type TransactionAccount = (Pubkey, AccountSharedData);
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Debug)]
 pub struct TransactionAccounts {
     accounts: Vec<RefCell<AccountSharedData>>,
     touched_flags: RefCell<Box<[bool]>>,
-    resize_delta: RefCell<i64>,
-    lamports_delta: RefCell<i128>,
+    resize_delta: Cell<i64>,
+    lamports_delta: Cell<i128>,
 }
 
 impl TransactionAccounts {
@@ -119,8 +121,8 @@ impl TransactionAccounts {
         TransactionAccounts {
             accounts,
             touched_flags: RefCell::new(touched_flags),
-            resize_delta: RefCell::new(0),
-            lamports_delta: RefCell::new(0),
+            resize_delta: Cell::new(0),
+            lamports_delta: Cell::new(0),
         }
     }
 
@@ -143,12 +145,10 @@ impl TransactionAccounts {
         old_len: usize,
         new_len: usize,
     ) -> Result<(), InstructionError> {
-        let mut accounts_resize_delta = self
-            .resize_delta
-            .try_borrow_mut()
-            .map_err(|_| InstructionError::GenericError)?;
-        *accounts_resize_delta =
-            accounts_resize_delta.saturating_add((new_len as i64).saturating_sub(old_len as i64));
+        let accounts_resize_delta = self.resize_delta.get();
+        self.resize_delta.set(
+            accounts_resize_delta.saturating_add((new_len as i64).saturating_sub(old_len as i64)),
+        );
         Ok(())
     }
 
@@ -159,12 +159,7 @@ impl TransactionAccounts {
         }
         // The resize can not exceed the per-transaction maximum
         let length_delta = (new_len as i64).saturating_sub(old_len as i64);
-        if self
-            .resize_delta
-            .try_borrow()
-            .map_err(|_| InstructionError::GenericError)
-            .map(|value_ref| *value_ref)?
-            .saturating_add(length_delta)
+        if self.resize_delta.get().saturating_add(length_delta)
             > MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION
         {
             return Err(InstructionError::MaxAccountsDataAllocationsExceeded);
@@ -172,6 +167,7 @@ impl TransactionAccounts {
         Ok(())
     }
 
+    #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
     fn try_borrow_mut(
         &self,
         index: IndexOfAccount,
@@ -195,22 +191,24 @@ impl TransactionAccounts {
     }
 
     fn add_lamports_delta(&self, balance: i128) -> Result<(), InstructionError> {
-        let mut delta = self.lamports_delta.borrow_mut();
-        *delta = delta
-            .checked_add(balance)
-            .ok_or(InstructionError::ArithmeticOverflow)?;
+        let delta = self.lamports_delta.get();
+        self.lamports_delta.set(
+            delta
+                .checked_add(balance)
+                .ok_or(InstructionError::ArithmeticOverflow)?,
+        );
         Ok(())
     }
 
     fn get_lamports_delta(&self) -> i128 {
-        *self.lamports_delta.borrow()
+        self.lamports_delta.get()
     }
 }
 
 /// Loaded transaction shared between runtime and programs.
 ///
 /// This context is valid for the entire duration of a transaction being processed.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug)]
 pub struct TransactionContext {
     account_keys: Pin<Box<[Pubkey]>>,
     accounts: Rc<TransactionAccounts>,
@@ -493,12 +491,8 @@ impl TransactionContext {
     }
 
     /// Returns the accounts resize delta
-    pub fn accounts_resize_delta(&self) -> Result<i64, InstructionError> {
-        self.accounts
-            .resize_delta
-            .try_borrow()
-            .map_err(|_| InstructionError::GenericError)
-            .map(|value_ref| *value_ref)
+    pub fn accounts_resize_delta(&self) -> i64 {
+        self.accounts.resize_delta.get()
     }
 
     /// Returns a new account data write access handler
@@ -539,16 +533,10 @@ impl TransactionContext {
                     debug_assert!(false);
                     return;
                 }
-                let Ok(remaining_allowed_growth) =
-                    accounts.resize_delta.try_borrow().map(|resize_delta| {
-                        MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION
-                            .saturating_sub(*resize_delta)
-                            .max(0) as usize
-                    })
-                else {
-                    debug_assert!(false);
-                    return;
-                };
+
+                let remaining_allowed_growth = MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION
+                    .saturating_sub(accounts.resize_delta.get())
+                    .max(0) as usize;
 
                 if requested_length > region.len as usize {
                     // Realloc immediately here to fit the requested access,
@@ -594,7 +582,7 @@ pub struct TransactionReturnData {
 /// Loaded instruction shared between runtime and programs.
 ///
 /// This context is valid for the entire duration of a (possibly cross program) instruction being processed.
-#[derive(Debug, Clone, Default, Eq, PartialEq)]
+#[derive(Debug, Clone, Default)]
 pub struct InstructionContext {
     nesting_level: usize,
     program_account_index_in_tx: IndexOfAccount,
@@ -724,50 +712,43 @@ impl InstructionContext {
             })
     }
 
-    fn try_borrow_account<'a, 'b: 'a>(
+    /// Get the owner of the program account of this instruction
+    pub fn get_program_owner(
+        &self,
+        transaction_context: &TransactionContext,
+    ) -> Result<Pubkey, InstructionError> {
+        self.get_index_of_program_account_in_transaction()
+            .and_then(|index_in_transaction| {
+                transaction_context
+                    .accounts
+                    .try_borrow(index_in_transaction)
+            })
+            .map(|acc| *acc.owner())
+    }
+
+    /// Gets an instruction account of this Instruction
+    pub fn try_borrow_instruction_account<'a, 'b: 'a>(
         &'a self,
         transaction_context: &'b TransactionContext,
-        index_in_transaction: IndexOfAccount,
-        index_in_instruction: Option<IndexOfAccount>,
+        index_in_instruction: IndexOfAccount,
     ) -> Result<BorrowedAccount<'a>, InstructionError> {
+        let instruction_account = *self
+            .instruction_accounts
+            .get(index_in_instruction as usize)
+            .ok_or(InstructionError::NotEnoughAccountKeys)?;
+
         let account = transaction_context
             .accounts
-            .try_borrow_mut(index_in_transaction)?;
+            .try_borrow_mut(instruction_account.index_in_transaction)?;
+
         Ok(BorrowedAccount {
             transaction_context,
-            instruction_context: self,
-            index_in_transaction,
-            index_in_instruction_accounts: index_in_instruction,
+            instruction_account,
             account,
+            index_in_transaction_of_instruction_program: self.program_account_index_in_tx,
         })
     }
 
-    /// Gets the last program account of this Instruction
-    pub fn try_borrow_program_account<'a, 'b: 'a>(
-        &'a self,
-        transaction_context: &'b TransactionContext,
-    ) -> Result<BorrowedAccount<'a>, InstructionError> {
-        let index_in_transaction = self.get_index_of_program_account_in_transaction()?;
-        let result = self.try_borrow_account(transaction_context, index_in_transaction, None);
-        debug_assert!(result.is_ok());
-        result
-    }
-
-    /// Gets an instruction account of this Instruction
-    pub fn try_borrow_instruction_account<'a, 'b: 'a>(
-        &'a self,
-        transaction_context: &'b TransactionContext,
-        instruction_account_index: IndexOfAccount,
-    ) -> Result<BorrowedAccount<'a>, InstructionError> {
-        let index_in_transaction =
-            self.get_index_of_instruction_account_in_transaction(instruction_account_index)?;
-        self.try_borrow_account(
-            transaction_context,
-            index_in_transaction,
-            Some(instruction_account_index),
-        )
-    }
-
     /// Returns whether an instruction account is a signer
     pub fn is_instruction_account_signer(
         &self,
@@ -827,25 +808,23 @@ impl InstructionContext {
 #[derive(Debug)]
 pub struct BorrowedAccount<'a> {
     transaction_context: &'a TransactionContext,
-    instruction_context: &'a InstructionContext,
-    index_in_transaction: IndexOfAccount,
-    // Program accounts are not part of the instruction_accounts vector, and thus None
-    index_in_instruction_accounts: Option<IndexOfAccount>,
     account: RefMut<'a, AccountSharedData>,
+    instruction_account: InstructionAccount,
+    index_in_transaction_of_instruction_program: IndexOfAccount,
 }
 
 impl BorrowedAccount<'_> {
     /// Returns the index of this account (transaction wide)
     #[inline]
     pub fn get_index_in_transaction(&self) -> IndexOfAccount {
-        self.index_in_transaction
+        self.instruction_account.index_in_transaction
     }
 
     /// Returns the public key of this account (transaction wide)
     #[inline]
     pub fn get_key(&self) -> &Pubkey {
         self.transaction_context
-            .get_key_of_account_at_index(self.index_in_transaction)
+            .get_key_of_account_at_index(self.instruction_account.index_in_transaction)
             .unwrap()
     }
 
@@ -1101,31 +1080,19 @@ impl BorrowedAccount<'_> {
 
     /// Returns whether this account is a signer (instruction wide)
     pub fn is_signer(&self) -> bool {
-        if let Some(index_in_instruction_accounts) = self.index_in_instruction_accounts {
-            self.instruction_context
-                .is_instruction_account_signer(index_in_instruction_accounts)
-                .unwrap_or_default()
-        } else {
-            false
-        }
+        self.instruction_account.is_signer()
     }
 
     /// Returns whether this account is writable (instruction wide)
     pub fn is_writable(&self) -> bool {
-        if let Some(index_in_instruction_accounts) = self.index_in_instruction_accounts {
-            self.instruction_context
-                .is_instruction_account_writable(index_in_instruction_accounts)
-                .unwrap_or_default()
-        } else {
-            false
-        }
+        self.instruction_account.is_writable()
     }
 
     /// Returns true if the owner of this account is the current `InstructionContext`s last program (instruction wide)
     pub fn is_owned_by_current_program(&self) -> bool {
-        self.instruction_context
-            .get_program_key(self.transaction_context)
-            .map(|key| key == self.get_owner())
+        self.transaction_context
+            .get_key_of_account_at_index(self.index_in_transaction_of_instruction_program)
+            .map(|program_key| program_key == self.get_owner())
             .unwrap_or_default()
     }
 
@@ -1161,7 +1128,7 @@ impl BorrowedAccount<'_> {
     fn touch(&self) -> Result<(), InstructionError> {
         self.transaction_context
             .accounts
-            .touch(self.index_in_transaction)
+            .touch(self.instruction_account.index_in_transaction)
     }
 
     #[cfg(not(target_os = "solana"))]
@@ -1206,7 +1173,7 @@ impl From<TransactionContext> for ExecutionRecord {
             accounts,
             return_data: context.return_data,
             touched_account_count,
-            accounts_resize_delta: RefCell::into_inner(resize_delta),
+            accounts_resize_delta: Cell::into_inner(resize_delta),
         }
     }
 }
@@ -1297,7 +1264,7 @@ mod tests {
         let result = instruction_context.get_program_key(&transaction_context);
         assert_eq!(result, Err(InstructionError::NotEnoughAccountKeys));
 
-        let result = instruction_context.try_borrow_program_account(&transaction_context);
+        let result = instruction_context.get_program_owner(&transaction_context);
         assert_eq!(result.err(), Some(InstructionError::NotEnoughAccountKeys));
     }
 }