Refactor - `TransactionProcessingCallback` (#7381)

* Adds Slot to return type of TransactionProcessingCallback::get_account_shared_data().

* Removes TransactionProcessingCallback::account_matches_owners().

* Adds comment in AccountLoader::get_account_shared_data().

Cargo.lock

@@ -10948,6 +10948,7 @@ name = "solana-svm-callback"
 version = "3.0.0"
 dependencies = [
  "solana-account",
+ "solana-clock",
  "solana-precompile-error",
  "solana-pubkey",
 ]

programs/sbf/Cargo.lock

@@ -9271,6 +9271,7 @@ name = "solana-svm-callback"
 version = "3.0.0"
 dependencies = [
  "solana-account",
+ "solana-clock",
  "solana-precompile-error",
  "solana-pubkey",
 ]

runtime/src/bank.rs

@@ -5666,20 +5666,11 @@ impl InvokeContextCallback for Bank {
 }
 
 impl TransactionProcessingCallback for Bank {
-    fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize> {
-        self.rc
-            .accounts
-            .accounts_db
-            .account_matches_owners(&self.ancestors, account, owners)
-            .ok()
-    }
-
-    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
+    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)> {
         self.rc
             .accounts
             .accounts_db
             .load_with_fixed_root(&self.ancestors, pubkey)
-            .map(|(acc, _)| acc)
     }
 
     // NOTE: must hold idempotent for the same set of arguments

svm-callback/Cargo.toml

@@ -11,6 +11,7 @@ readme = false
 
 [dependencies]
 solana-account = { workspace = true }
+solana-clock = { workspace = true }
 solana-precompile-error = { workspace = true }
 solana-pubkey = { workspace = true }
 

svm-callback/src/lib.rs

@@ -1,6 +1,6 @@
 use {
-    solana_account::AccountSharedData, solana_precompile_error::PrecompileError,
-    solana_pubkey::Pubkey,
+    solana_account::AccountSharedData, solana_clock::Slot,
+    solana_precompile_error::PrecompileError, solana_pubkey::Pubkey,
 };
 
 /// Callback used by InvokeContext in SVM
@@ -33,9 +33,7 @@ pub trait InvokeContextCallback {
 
 /// Runtime callbacks for transaction processing.
 pub trait TransactionProcessingCallback: InvokeContextCallback {
-    fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize>;
-
-    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData>;
+    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)>;
 
     fn add_builtin_account(&self, _name: &str, _program_id: &Pubkey) {}
 

svm/doc/spec.md

@@ -128,9 +128,7 @@ information.
 
 ```rust
 pub trait TransactionProcessingCallback {
-    fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize>;
-
-    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData>;
+    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)>;
 
     fn add_builtin_account(&self, _name: &str, _program_id: &Pubkey) {}
 

svm/src/account_loader.rs

@@ -14,6 +14,7 @@ use {
         state_traits::StateMut, Account, AccountSharedData, ReadableAccount, WritableAccount,
         PROGRAM_OWNERS,
     },
+    solana_clock::Slot,
     solana_fee_structure::FeeDetails,
     solana_instruction::{BorrowedAccountMeta, BorrowedInstruction},
     solana_instructions_sysvar::construct_instructions_data,
@@ -258,7 +259,7 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
             };
 
             (option_account, false)
-        } else if let Some(account) = self.callbacks.get_account_shared_data(account_key) {
+        } else if let Some((account, _slot)) = self.callbacks.get_account_shared_data(account_key) {
             (Some(account), true)
         } else {
             (None, false)
@@ -318,14 +319,10 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> {
 // In general, most accounts we load this way should already be in our accounts store.
 // Once SIMD-0186 is implemented, 100% of accounts will be.
 impl<CB: TransactionProcessingCallback> TransactionProcessingCallback for AccountLoader<'_, CB> {
-    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
-        self.do_load(pubkey).0
-    }
-
-    fn account_matches_owners(&self, pubkey: &Pubkey, owners: &[Pubkey]) -> Option<usize> {
-        self.do_load(pubkey)
-            .0
-            .and_then(|account| owners.iter().position(|entry| entry == account.owner()))
+    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)> {
+        // The returned last-modification-slot is a dummy value for now,
+        // but will later be used in IndexImplementation::V2 of the global program cache.
+        self.do_load(pubkey).0.map(|account| (account, 0))
     }
 }
 
@@ -890,12 +887,10 @@ mod tests {
     impl InvokeContextCallback for TestCallbacks {}
 
     impl TransactionProcessingCallback for TestCallbacks {
-        fn account_matches_owners(&self, _account: &Pubkey, _owners: &[Pubkey]) -> Option<usize> {
-            None
-        }
-
-        fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
-            self.accounts_map.get(pubkey).cloned()
+        fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)> {
+            self.accounts_map
+                .get(pubkey)
+                .map(|account| (account.clone(), 0))
         }
 
         fn inspect_account(
@@ -2758,7 +2753,10 @@ mod tests {
 
         let account_loader: AccountLoader<_> = (&mock_bank).into();
         assert_eq!(
-            account_loader.get_account_shared_data(&fee_payer).unwrap(),
+            account_loader
+                .get_account_shared_data(&fee_payer)
+                .unwrap()
+                .0,
             fee_payer_account
         );
 
@@ -2785,7 +2783,10 @@ mod tests {
             fee_payer_account
         );
         assert_eq!(
-            account_loader.get_account_shared_data(&fee_payer).unwrap(),
+            account_loader
+                .get_account_shared_data(&fee_payer)
+                .unwrap()
+                .0,
             fee_payer_account
         );
 

svm/src/program_loader.rs

@@ -64,7 +64,7 @@ pub(crate) fn load_program_accounts<CB: TransactionProcessingCallback>(
     callbacks: &CB,
     pubkey: &Pubkey,
 ) -> Option<ProgramAccountLoadResult> {
-    let program_account = callbacks.get_account_shared_data(pubkey)?;
+    let (program_account, _slot) = callbacks.get_account_shared_data(pubkey)?;
 
     if loader_v4::check_id(program_account.owner()) {
         return Some(
@@ -92,7 +92,9 @@ pub(crate) fn load_program_accounts<CB: TransactionProcessingCallback>(
         programdata_address,
     }) = program_account.state()
     {
-        if let Some(programdata_account) = callbacks.get_account_shared_data(&programdata_address) {
+        if let Some((programdata_account, _slot)) =
+            callbacks.get_account_shared_data(&programdata_address)
+        {
             if let Ok(UpgradeableLoaderState::ProgramData {
                 slot,
                 upgrade_authority_address: _,
@@ -217,7 +219,7 @@ pub(crate) fn get_program_modification_slot<CB: TransactionProcessingCallback>(
     callbacks: &CB,
     pubkey: &Pubkey,
 ) -> TransactionResult<Slot> {
-    let program = callbacks
+    let (program, _slot) = callbacks
         .get_account_shared_data(pubkey)
         .ok_or(TransactionError::ProgramAccountNotFound)?;
     if bpf_loader_upgradeable::check_id(program.owner()) {
@@ -225,7 +227,7 @@ pub(crate) fn get_program_modification_slot<CB: TransactionProcessingCallback>(
             programdata_address,
         }) = program.state()
         {
-            let programdata = callbacks
+            let (programdata, _slot) = callbacks
                 .get_account_shared_data(&programdata_address)
                 .ok_or(TransactionError::ProgramAccountNotFound)?;
             if let Ok(UpgradeableLoaderState::ProgramData {
@@ -283,20 +285,11 @@ mod tests {
     impl InvokeContextCallback for MockBankCallback {}
 
     impl TransactionProcessingCallback for MockBankCallback {
-        fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize> {
-            if let Some(data) = self.account_shared_data.borrow().get(account) {
-                if data.lamports() == 0 {
-                    None
-                } else {
-                    owners.iter().position(|entry| data.owner() == entry)
-                }
-            } else {
-                None
-            }
-        }
-
-        fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
-            self.account_shared_data.borrow().get(pubkey).cloned()
+        fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)> {
+            self.account_shared_data
+                .borrow()
+                .get(pubkey)
+                .map(|account| (account.clone(), 0))
         }
 
         fn add_builtin_account(&self, name: &str, program_id: &Pubkey) {

svm/src/transaction_processor.rs

@@ -712,13 +712,13 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             if let Some(cache_entry) = program_cache_for_tx_batch.find(account_key) {
                 cache_entry.tx_usage_counter.fetch_add(1, Ordering::Relaxed);
             } else if account_loader
-                .account_matches_owners(account_key, PROGRAM_OWNERS)
-                .is_some()
+                .get_account_shared_data(account_key)
+                .map(|(account, _slot)| PROGRAM_OWNERS.contains(account.owner()))
+                .unwrap_or(false)
             {
                 program_accounts_set.insert(*account_key);
             }
         }
-
         program_accounts_set
     }
 
@@ -1040,7 +1040,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
     ) {
         let mut sysvar_cache = self.sysvar_cache.write().unwrap();
         sysvar_cache.fill_missing_entries(|pubkey, set_sysvar| {
-            if let Some(account) = callbacks.get_account_shared_data(pubkey) {
+            if let Some((account, _slot)) = callbacks.get_account_shared_data(pubkey) {
                 set_sysvar(account.data());
             }
         });
@@ -1160,24 +1160,12 @@ mod tests {
     impl InvokeContextCallback for MockBankCallback {}
 
     impl TransactionProcessingCallback for MockBankCallback {
-        fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize> {
-            if let Some(data) = self.account_shared_data.read().unwrap().get(account) {
-                if data.lamports() == 0 {
-                    None
-                } else {
-                    owners.iter().position(|entry| data.owner() == entry)
-                }
-            } else {
-                None
-            }
-        }
-
-        fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
+        fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)> {
             self.account_shared_data
                 .read()
                 .unwrap()
                 .get(pubkey)
-                .cloned()
+                .map(|account| (account.clone(), 0))
         }
 
         fn add_builtin_account(&self, name: &str, program_id: &Pubkey) {

svm/tests/mock_bank.rs

@@ -68,24 +68,12 @@ pub struct MockBankCallback {
 impl InvokeContextCallback for MockBankCallback {}
 
 impl TransactionProcessingCallback for MockBankCallback {
-    fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option<usize> {
-        if let Some(data) = self.account_shared_data.read().unwrap().get(account) {
-            if data.lamports() == 0 {
-                None
-            } else {
-                owners.iter().position(|entry| data.owner() == entry)
-            }
-        } else {
-            None
-        }
-    }
-
-    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<AccountSharedData> {
+    fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option<(AccountSharedData, Slot)> {
         self.account_shared_data
             .read()
             .unwrap()
             .get(pubkey)
-            .cloned()
+            .map(|account| (account.clone(), 0))
     }
 
     fn add_builtin_account(&self, name: &str, program_id: &Pubkey) {