Refactor: Add trait for loading addresses (#22903)

core/src/banking_stage.rs

@@ -37,15 +37,14 @@ use {
             MAX_TRANSACTION_FORWARDING_DELAY_GPU,
         },
         feature_set,
-        message::{
+        message::Message,
-            v0::{LoadedAddresses, MessageAddressTableLookup},
-            Message,
-        },
         pubkey::Pubkey,
         short_vec::decode_shortu16_len,
         signature::Signature,
         timing::{duration_as_ms, timestamp, AtomicInterval},
-        transaction::{self, SanitizedTransaction, TransactionError, VersionedTransaction},
+        transaction::{
+            self, AddressLoader, SanitizedTransaction, TransactionError, VersionedTransaction,
+        },
     },
     solana_streamer::sendmmsg::{batch_send, SendPktsError},
     solana_transaction_status::token_balances::{
@@ -1394,7 +1393,7 @@ impl BankingStage {
         transaction_indexes: &[usize],
         feature_set: &Arc<feature_set::FeatureSet>,
         votes_only: bool,
-        address_loader: impl Fn(&[MessageAddressTableLookup]) -> transaction::Result<LoadedAddresses>,
+        address_loader: &impl AddressLoader,
     ) -> (Vec<SanitizedTransaction>, Vec<usize>) {
         transaction_indexes
             .iter()
@@ -1411,7 +1410,7 @@ impl BankingStage {
                     tx,
                     message_hash,
                     Some(p.meta.is_simple_vote_tx()),
-                    &address_loader,
+                    address_loader,
                 )
                 .ok()?;
                 tx.verify_precompiles(feature_set).ok()?;
@@ -1477,7 +1476,7 @@ impl BankingStage {
             &packet_indexes,
             &bank.feature_set,
             bank.vote_only_bank(),
-            |lookup| bank.load_lookup_table_addresses(lookup),
+            bank.as_ref(),
         );
         packet_conversion_time.stop();
         inc_new_counter_info!("banking_stage-packet_conversion", 1);
@@ -1555,7 +1554,7 @@ impl BankingStage {
             transaction_indexes,
             &bank.feature_set,
             bank.vote_only_bank(),
-            |lookup| bank.load_lookup_table_addresses(lookup),
+            bank.as_ref(),
         );
         unprocessed_packet_conversion_time.stop();
 
@@ -1778,12 +1777,15 @@ mod tests {
             account::AccountSharedData,
             hash::Hash,
             instruction::InstructionError,
-            message::{v0, MessageHeader, VersionedMessage},
+            message::{
+                v0::{self, MessageAddressTableLookup},
+                MessageHeader, VersionedMessage,
+            },
             poh_config::PohConfig,
             signature::{Keypair, Signer},
             system_instruction::SystemError,
             system_transaction,
-            transaction::{Transaction, TransactionError},
+            transaction::{DisabledAddressLoader, Transaction, TransactionError},
         },
         solana_streamer::{recvmmsg::recv_mmsg, socket::SocketAddrSpace},
         solana_transaction_status::{TransactionStatusMeta, VersionedTransactionWithStatusMeta},
@@ -3111,15 +3113,12 @@ mod tests {
         let tx = VersionedTransaction::try_new(message, &[&keypair]).unwrap();
         let message_hash = tx.message.hash();
         let sanitized_tx =
-            SanitizedTransaction::try_create(tx.clone(), message_hash, Some(false), |lookups| {
+            SanitizedTransaction::try_create(tx.clone(), message_hash, Some(false), bank.as_ref())
-                Ok(bank.load_lookup_table_addresses(lookups).unwrap())
+                .unwrap();
-            })
-            .unwrap();
 
         let entry = next_versioned_entry(&genesis_config.hash(), 1, vec![tx]);
         let entries = vec![entry];
 
-        // todo: check if sig fees are needed
         bank.transfer(1, &mint_keypair, &keypair.pubkey()).unwrap();
 
         let ledger_path = get_tmp_ledger_path_auto_delete!();
@@ -3753,7 +3752,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                |_| Err(TransactionError::UnsupportedVersion),
+                &DisabledAddressLoader,
             );
             assert_eq!(2, txs.len());
             assert_eq!(vec![0, 1], tx_packet_index);
@@ -3764,7 +3763,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                |_| Err(TransactionError::UnsupportedVersion),
+                &DisabledAddressLoader,
             );
             assert_eq!(0, txs.len());
             assert_eq!(0, tx_packet_index.len());
@@ -3784,7 +3783,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                |_| Err(TransactionError::UnsupportedVersion),
+                &DisabledAddressLoader,
             );
             assert_eq!(3, txs.len());
             assert_eq!(vec![0, 1, 2], tx_packet_index);
@@ -3795,7 +3794,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                |_| Err(TransactionError::UnsupportedVersion),
+                &DisabledAddressLoader,
             );
             assert_eq!(2, txs.len());
             assert_eq!(vec![0, 2], tx_packet_index);
@@ -3815,7 +3814,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                |_| Err(TransactionError::UnsupportedVersion),
+                &DisabledAddressLoader,
             );
             assert_eq!(3, txs.len());
             assert_eq!(vec![0, 1, 2], tx_packet_index);
@@ -3826,7 +3825,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                |_| Err(TransactionError::UnsupportedVersion),
+                &DisabledAddressLoader,
             );
             assert_eq!(3, txs.len());
             assert_eq!(vec![0, 1, 2], tx_packet_index);

entry/benches/entry_sigverify.rs

@@ -6,7 +6,7 @@ use {
     solana_sdk::{
         hash::Hash,
         transaction::{
-            Result, SanitizedTransaction, TransactionError, TransactionVerificationMode,
+            DisabledAddressLoader, Result, SanitizedTransaction, TransactionVerificationMode,
             VersionedTransaction,
         },
     },
@@ -35,9 +35,12 @@ fn bench_gpusigverify(bencher: &mut Bencher) {
                         versioned_tx.message.hash()
                     };
 
-                SanitizedTransaction::try_create(versioned_tx, message_hash, None, |_| {
+                SanitizedTransaction::try_create(
-                    Err(TransactionError::UnsupportedVersion)
+                    versioned_tx,
-                })
+                    message_hash,
+                    None,
+                    &DisabledAddressLoader,
+                )
             }?;
 
             Ok(sanitized_tx)
@@ -73,10 +76,12 @@ fn bench_cpusigverify(bencher: &mut Bencher) {
         move |versioned_tx: VersionedTransaction| -> Result<SanitizedTransaction> {
             let sanitized_tx = {
                 let message_hash = versioned_tx.verify_and_hash_message()?;
-
+                SanitizedTransaction::try_create(
-                SanitizedTransaction::try_create(versioned_tx, message_hash, None, |_| {
+                    versioned_tx,
-                    Err(TransactionError::UnsupportedVersion)
+                    message_hash,
-                })
+                    None,
+                    &DisabledAddressLoader,
+                )
             }?;
 
             Ok(sanitized_tx)

entry/src/entry.rs

@@ -932,7 +932,9 @@ mod tests {
             pubkey::Pubkey,
             signature::{Keypair, Signer},
             system_transaction,
-            transaction::{Result, SanitizedTransaction, TransactionError, VersionedTransaction},
+            transaction::{
+                DisabledAddressLoader, Result, SanitizedTransaction, VersionedTransaction,
+            },
         },
     };
 
@@ -1016,9 +1018,12 @@ mod tests {
                             versioned_tx.message.hash()
                         };
 
-                    SanitizedTransaction::try_create(versioned_tx, message_hash, None, |_| {
+                    SanitizedTransaction::try_create(
-                        Err(TransactionError::UnsupportedVersion)
+                        versioned_tx,
-                    })
+                        message_hash,
+                        None,
+                        &DisabledAddressLoader,
+                    )
                 }?;
 
                 Ok(sanitized_tx)

ledger-tool/src/main.rs

@@ -56,7 +56,7 @@ use {
         shred_version::compute_shred_version,
         stake::{self, state::StakeState},
         system_program,
-        transaction::{SanitizedTransaction, TransactionError},
+        transaction::{DisabledAddressLoader, SanitizedTransaction},
     },
     solana_stake_program::stake_state::{self, PointValue},
     solana_vote_program::{
@@ -232,10 +232,12 @@ fn output_slot(
             num_hashes += entry.num_hashes;
             for transaction in entry.transactions {
                 let tx_signature = transaction.signatures[0];
-                let sanitize_result =
+                let sanitize_result = SanitizedTransaction::try_create(
-                    SanitizedTransaction::try_create(transaction, Hash::default(), None, |_| {
+                    transaction,
-                        Err(TransactionError::UnsupportedVersion)
+                    Hash::default(),
-                    });
+                    None,
+                    &DisabledAddressLoader,
+                );
 
                 match sanitize_result {
                     Ok(transaction) => {
@@ -780,9 +782,12 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
             .transactions
             .into_iter()
             .filter_map(|transaction| {
-                SanitizedTransaction::try_create(transaction, Hash::default(), None, |_| {
+                SanitizedTransaction::try_create(
-                    Err(TransactionError::UnsupportedVersion)
+                    transaction,
-                })
+                    Hash::default(),
+                    None,
+                    &DisabledAddressLoader,
+                )
                 .map_err(|err| {
                     warn!("Failed to compute cost of transaction: {:?}", err);
                 })

rpc/src/rpc.rs

@@ -68,7 +68,10 @@ use {
         stake_history::StakeHistory,
         system_instruction,
         sysvar::stake_history,
-        transaction::{self, SanitizedTransaction, TransactionError, VersionedTransaction},
+        transaction::{
+            self, DisabledAddressLoader, SanitizedTransaction, TransactionError,
+            VersionedTransaction,
+        },
     },
     solana_send_transaction_service::{
         send_transaction_service::{SendTransactionService, TransactionInfo},
@@ -4271,10 +4274,8 @@ where
 
 fn sanitize_transaction(transaction: VersionedTransaction) -> Result<SanitizedTransaction> {
     let message_hash = transaction.message.hash();
-    SanitizedTransaction::try_create(transaction, message_hash, None, |_| {
+    SanitizedTransaction::try_create(transaction, message_hash, None, &DisabledAddressLoader)
-        Err(TransactionError::UnsupportedVersion)
+        .map_err(|err| Error::invalid_params(format!("invalid transaction: {}", err)))
-    })
-    .map_err(|err| Error::invalid_params(format!("invalid transaction: {}", err)))
 }
 
 pub(crate) fn create_validator_exit(exit: &Arc<AtomicBool>) -> Arc<RwLock<Exit>> {

rpc/src/transaction_status_service.rs

@@ -216,7 +216,7 @@ pub(crate) mod tests {
             signature::{Keypair, Signature, Signer},
             system_transaction,
             transaction::{
-                SanitizedTransaction, Transaction, TransactionError, VersionedTransaction,
+                DisabledAddressLoader, SanitizedTransaction, Transaction, VersionedTransaction,
             },
         },
         solana_transaction_status::{
@@ -298,11 +298,13 @@ pub(crate) mod tests {
         let message_hash = Hash::new_unique();
         let transaction = build_test_transaction_legacy();
         let transaction = VersionedTransaction::from(transaction);
-        let transaction =
+        let transaction = SanitizedTransaction::try_create(
-            SanitizedTransaction::try_create(transaction, message_hash, Some(true), |_| {
+            transaction,
-                Err(TransactionError::UnsupportedVersion)
+            message_hash,
-            })
+            Some(true),
-            .unwrap();
+            &DisabledAddressLoader,
+        )
+        .unwrap();
 
         let expected_transaction = transaction.clone();
         let pubkey = Pubkey::new_unique();

runtime/src/bank.rs

@@ -109,10 +109,7 @@ use {
         inflation::Inflation,
         instruction::CompiledInstruction,
         lamports::LamportsError,
-        message::{
+        message::SanitizedMessage,
-            v0::{LoadedAddresses, MessageAddressTableLookup},
-            SanitizedMessage,
-        },
         native_loader,
         native_token::sol_to_lamports,
         nonce, nonce_account,
@@ -127,7 +124,7 @@ use {
         sysvar::{self, Sysvar, SysvarId},
         timing::years_as_slots,
         transaction::{
-            AddressLookupError, Result, SanitizedTransaction, Transaction, TransactionError,
+            Result, SanitizedTransaction, Transaction, TransactionError,
             TransactionVerificationMode, VersionedTransaction,
         },
         transaction_context::{InstructionTrace, TransactionAccount, TransactionContext},
@@ -157,6 +154,7 @@ use {
     },
 };
 
+mod address_lookup_table;
 mod sysvar_cache;
 mod transaction_account_state_info;
 
@@ -3393,9 +3391,7 @@ impl Bank {
             .into_iter()
             .map(|tx| {
                 let message_hash = tx.message.hash();
-                SanitizedTransaction::try_create(tx, message_hash, None, |_| {
+                SanitizedTransaction::try_create(tx, message_hash, None, self)
-                    Err(TransactionError::UnsupportedVersion)
-                })
             })
             .collect::<Result<Vec<_>>>()?;
         let lock_results = self
@@ -3796,33 +3792,6 @@ impl Bank {
         Arc::make_mut(&mut cache).remove(pubkey);
     }
 
-    pub fn load_lookup_table_addresses(
-        &self,
-        address_table_lookups: &[MessageAddressTableLookup],
-    ) -> Result<LoadedAddresses> {
-        if !self.versioned_tx_message_enabled() {
-            return Err(TransactionError::UnsupportedVersion);
-        }
-
-        let slot_hashes = self
-            .sysvar_cache
-            .read()
-            .unwrap()
-            .get_slot_hashes()
-            .map_err(|_| TransactionError::AccountNotFound)?;
-
-        Ok(address_table_lookups
-            .iter()
-            .map(|address_table_lookup| {
-                self.rc.accounts.load_lookup_table_addresses(
-                    &self.ancestors,
-                    address_table_lookup,
-                    &slot_hashes,
-                )
-            })
-            .collect::<std::result::Result<_, AddressLookupError>>()?)
-    }
-
     /// Execute a transaction using the provided loaded accounts and update
     /// the executors cache if the transaction was successful.
     fn execute_loaded_transaction(
@@ -5760,9 +5729,7 @@ impl Bank {
                 tx.message.hash()
             };
 
-            SanitizedTransaction::try_create(tx, message_hash, None, |lookups| {
+            SanitizedTransaction::try_create(tx, message_hash, None, self)
-                self.load_lookup_table_addresses(lookups)
-            })
         }?;
 
         if verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles

runtime/src/bank/address_lookup_table.rs

@@ -0,0 +1,38 @@
+use {
+    super::Bank,
+    solana_sdk::{
+        message::v0::{LoadedAddresses, MessageAddressTableLookup},
+        transaction::{
+            AddressLoader, AddressLookupError, Result as TransactionResult, TransactionError,
+        },
+    },
+};
+
+impl AddressLoader for Bank {
+    fn load_addresses(
+        &self,
+        address_table_lookups: &[MessageAddressTableLookup],
+    ) -> TransactionResult<LoadedAddresses> {
+        if !self.versioned_tx_message_enabled() {
+            return Err(TransactionError::UnsupportedVersion);
+        }
+
+        let slot_hashes = self
+            .sysvar_cache
+            .read()
+            .unwrap()
+            .get_slot_hashes()
+            .map_err(|_| TransactionError::AccountNotFound)?;
+
+        Ok(address_table_lookups
+            .iter()
+            .map(|address_table_lookup| {
+                self.rc.accounts.load_lookup_table_addresses(
+                    &self.ancestors,
+                    address_table_lookup,
+                    &slot_hashes,
+                )
+            })
+            .collect::<Result<_, AddressLookupError>>()?)
+    }
+}

runtime/src/cost_tracker.rs

@@ -236,7 +236,7 @@ mod tests {
             hash::Hash,
             signature::{Keypair, Signer},
             system_transaction,
-            transaction::{TransactionError, VersionedTransaction},
+            transaction::{DisabledAddressLoader, VersionedTransaction},
         },
         solana_vote_program::vote_transaction,
         std::{cmp, sync::Arc},
@@ -285,7 +285,7 @@ mod tests {
             VersionedTransaction::from(transaction),
             message_hash,
             Some(true),
-            |_| Err(TransactionError::UnsupportedVersion),
+            &DisabledAddressLoader,
         )
         .unwrap();
         (vote_transaction, vec![mint_keypair.pubkey()], 10)

sdk/src/transaction/sanitized.rs

@@ -39,6 +39,17 @@ pub struct TransactionAccountLocks<'a> {
     pub writable: Vec<&'a Pubkey>,
 }
 
+pub trait AddressLoader {
+    fn load_addresses(&self, lookups: &[MessageAddressTableLookup]) -> Result<LoadedAddresses>;
+}
+
+pub struct DisabledAddressLoader;
+impl AddressLoader for DisabledAddressLoader {
+    fn load_addresses(&self, _lookups: &[MessageAddressTableLookup]) -> Result<LoadedAddresses> {
+        Err(TransactionError::UnsupportedVersion)
+    }
+}
+
 impl SanitizedTransaction {
     /// Create a sanitized transaction from an unsanitized transaction.
     /// If the input transaction uses address tables, attempt to lookup
@@ -47,7 +58,7 @@ impl SanitizedTransaction {
         tx: VersionedTransaction,
         message_hash: Hash,
         is_simple_vote_tx: Option<bool>,
-        address_loader: impl Fn(&[MessageAddressTableLookup]) -> Result<LoadedAddresses>,
+        address_loader: &impl AddressLoader,
     ) -> Result<Self> {
         tx.sanitize()?;
 
@@ -55,7 +66,7 @@ impl SanitizedTransaction {
         let message = match tx.message {
             VersionedMessage::Legacy(message) => SanitizedMessage::Legacy(message),
             VersionedMessage::V0(message) => SanitizedMessage::V0(v0::LoadedMessage {
-                loaded_addresses: address_loader(&message.address_table_lookups)?,
+                loaded_addresses: address_loader.load_addresses(&message.address_table_lookups)?,
                 message,
             }),
         };