Refactor: Sanitized transaction creation (#23558)

* Refactor: SanitizedTransaction::try_create optionally computes hash

* Refactor: Add SimpleAddressLoader

core/src/banking_stage.rs

@@ -1680,7 +1680,7 @@ impl BankingStage {
         transaction_indexes: &[usize],
         feature_set: &Arc<feature_set::FeatureSet>,
         votes_only: bool,
-        address_loader: &impl AddressLoader,
+        address_loader: impl AddressLoader,
     ) -> (Vec<SanitizedTransaction>, Vec<usize>) {
         transaction_indexes
             .iter()
@@ -1697,7 +1697,7 @@ impl BankingStage {
                     tx,
                     message_hash,
                     Some(p.meta.is_simple_vote_tx()),
-                    address_loader,
+                    address_loader.clone(),
                 )
                 .ok()?;
                 tx.verify_precompiles(feature_set).ok()?;
@@ -2128,7 +2128,7 @@ mod tests {
             signature::{Keypair, Signer},
             system_instruction::SystemError,
             system_transaction,
-            transaction::{DisabledAddressLoader, Transaction, TransactionError},
+            transaction::{MessageHash, SimpleAddressLoader, Transaction, TransactionError},
         },
         solana_streamer::{recvmmsg::recv_mmsg, socket::SocketAddrSpace},
         solana_transaction_status::{TransactionStatusMeta, VersionedTransactionWithStatusMeta},
@@ -3464,10 +3464,13 @@ mod tests {
         });
 
         let tx = VersionedTransaction::try_new(message, &[&keypair]).unwrap();
-        let message_hash = tx.message.hash();
+        let sanitized_tx = SanitizedTransaction::try_create(
-        let sanitized_tx =
+            tx.clone(),
-            SanitizedTransaction::try_create(tx.clone(), message_hash, Some(false), bank.as_ref())
+            MessageHash::Compute,
-                .unwrap();
+            Some(false),
+            bank.as_ref(),
+        )
+        .unwrap();
 
         let entry = next_versioned_entry(&genesis_config.hash(), 1, vec![tx]);
         let entries = vec![entry];
@@ -4145,7 +4148,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                &DisabledAddressLoader,
+                SimpleAddressLoader::Disabled,
             );
             assert_eq!(2, txs.len());
             assert_eq!(vec![0, 1], tx_packet_index);
@@ -4156,7 +4159,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                &DisabledAddressLoader,
+                SimpleAddressLoader::Disabled,
             );
             assert_eq!(0, txs.len());
             assert_eq!(0, tx_packet_index.len());
@@ -4176,7 +4179,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                &DisabledAddressLoader,
+                SimpleAddressLoader::Disabled,
             );
             assert_eq!(3, txs.len());
             assert_eq!(vec![0, 1, 2], tx_packet_index);
@@ -4187,7 +4190,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                &DisabledAddressLoader,
+                SimpleAddressLoader::Disabled,
             );
             assert_eq!(2, txs.len());
             assert_eq!(vec![0, 2], tx_packet_index);
@@ -4207,7 +4210,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                &DisabledAddressLoader,
+                SimpleAddressLoader::Disabled,
             );
             assert_eq!(3, txs.len());
             assert_eq!(vec![0, 1, 2], tx_packet_index);
@@ -4218,7 +4221,7 @@ mod tests {
                 &packet_indexes,
                 &Arc::new(FeatureSet::default()),
                 votes_only,
-                &DisabledAddressLoader,
+                SimpleAddressLoader::Disabled,
             );
             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::{
-            DisabledAddressLoader, Result, SanitizedTransaction, TransactionVerificationMode,
+            Result, SanitizedTransaction, SimpleAddressLoader, TransactionVerificationMode,
             VersionedTransaction,
         },
     },
@@ -39,7 +39,7 @@ fn bench_gpusigverify(bencher: &mut Bencher) {
                     versioned_tx,
                     message_hash,
                     None,
-                    &DisabledAddressLoader,
+                    SimpleAddressLoader::Disabled,
                 )
             }?;
 
@@ -80,7 +80,7 @@ fn bench_cpusigverify(bencher: &mut Bencher) {
                     versioned_tx,
                     message_hash,
                     None,
-                    &DisabledAddressLoader,
+                    SimpleAddressLoader::Disabled,
                 )
             }?;
 

entry/src/entry.rs

@@ -933,7 +933,7 @@ mod tests {
             signature::{Keypair, Signer},
             system_transaction,
             transaction::{
-                DisabledAddressLoader, Result, SanitizedTransaction, VersionedTransaction,
+                Result, SanitizedTransaction, SimpleAddressLoader, VersionedTransaction,
             },
         },
     };
@@ -1022,7 +1022,7 @@ mod tests {
                         versioned_tx,
                         message_hash,
                         None,
-                        &DisabledAddressLoader,
+                        SimpleAddressLoader::Disabled,
                     )
                 }?;
 

ledger-tool/src/main.rs

@@ -60,7 +60,7 @@ use {
         shred_version::compute_shred_version,
         stake::{self, state::StakeState},
         system_program,
-        transaction::{DisabledAddressLoader, SanitizedTransaction},
+        transaction::{MessageHash, SanitizedTransaction, SimpleAddressLoader},
     },
     solana_stake_program::stake_state::{self, PointValue},
     solana_vote_program::{
@@ -242,9 +242,9 @@ fn output_slot(
                 let tx_signature = transaction.signatures[0];
                 let sanitize_result = SanitizedTransaction::try_create(
                     transaction,
-                    Hash::default(),
+                    MessageHash::Compute,
                     None,
-                    &DisabledAddressLoader,
+                    SimpleAddressLoader::Disabled,
                 );
 
                 match sanitize_result {
@@ -794,9 +794,9 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
             .filter_map(|transaction| {
                 SanitizedTransaction::try_create(
                     transaction,
-                    Hash::default(),
+                    MessageHash::Compute,
                     None,
-                    &DisabledAddressLoader,
+                    SimpleAddressLoader::Disabled,
                 )
                 .map_err(|err| {
                     warn!("Failed to compute cost of transaction: {:?}", err);

rpc/src/rpc.rs

@@ -70,7 +70,8 @@ use {
         system_instruction,
         sysvar::stake_history,
         transaction::{
-            self, AddressLoader, SanitizedTransaction, TransactionError, VersionedTransaction,
+            self, AddressLoader, MessageHash, SanitizedTransaction, TransactionError,
+            VersionedTransaction,
         },
     },
     solana_send_transaction_service::{
@@ -4277,10 +4278,9 @@ where
 
 fn sanitize_transaction(
     transaction: VersionedTransaction,
-    address_loader: &impl AddressLoader,
+    address_loader: impl AddressLoader,
 ) -> Result<SanitizedTransaction> {
-    let message_hash = transaction.message.hash();
+    SanitizedTransaction::try_create(transaction, MessageHash::Compute, None, address_loader)
-    SanitizedTransaction::try_create(transaction, message_hash, None, address_loader)
         .map_err(|err| Error::invalid_params(format!("invalid transaction: {}", err)))
 }
 
@@ -4420,7 +4420,7 @@ pub mod tests {
             system_program, system_transaction,
             timing::slot_duration_from_slots_per_year,
             transaction::{
-                self, DisabledAddressLoader, Transaction, TransactionError, TransactionVersion,
+                self, SimpleAddressLoader, Transaction, TransactionError, TransactionVersion,
             },
         },
         solana_transaction_status::{
@@ -7823,7 +7823,8 @@ pub mod tests {
                 .to_string(),
         );
         assert_eq!(
-            sanitize_transaction(unsanitary_versioned_tx, &DisabledAddressLoader).unwrap_err(),
+            sanitize_transaction(unsanitary_versioned_tx, SimpleAddressLoader::Disabled)
+                .unwrap_err(),
             expect58
         );
     }
@@ -7843,9 +7844,9 @@ pub mod tests {
         };
 
         assert_eq!(
-            sanitize_transaction(versioned_tx, &DisabledAddressLoader).unwrap_err(),
+            sanitize_transaction(versioned_tx, SimpleAddressLoader::Disabled).unwrap_err(),
             Error::invalid_params(
-                "invalid transaction: Transaction version is unsupported".to_string(),
+                "invalid transaction: Transaction loads an address table account that doesn't exist".to_string(),
             )
         );
     }

rpc/src/transaction_status_service.rs

@@ -225,7 +225,8 @@ pub(crate) mod tests {
             signature::{Keypair, Signature, Signer},
             system_transaction,
             transaction::{
-                DisabledAddressLoader, SanitizedTransaction, Transaction, VersionedTransaction,
+                MessageHash, SanitizedTransaction, SimpleAddressLoader, Transaction,
+                VersionedTransaction,
             },
         },
         solana_transaction_status::{
@@ -304,14 +305,13 @@ pub(crate) mod tests {
             Blockstore::open(&ledger_path).expect("Expected to be able to open database ledger");
         let blockstore = Arc::new(blockstore);
 
-        let message_hash = Hash::new_unique();
         let transaction = build_test_transaction_legacy();
         let transaction = VersionedTransaction::from(transaction);
         let transaction = SanitizedTransaction::try_create(
             transaction,
-            message_hash,
+            MessageHash::Compute,
-            Some(true),
+            None,
-            &DisabledAddressLoader,
+            SimpleAddressLoader::Disabled,
         )
         .unwrap();
 

runtime/src/bank.rs

@@ -127,7 +127,7 @@ use {
         sysvar::{self, Sysvar, SysvarId},
         timing::years_as_slots,
         transaction::{
-            Result, SanitizedTransaction, Transaction, TransactionError,
+            MessageHash, Result, SanitizedTransaction, Transaction, TransactionError,
             TransactionVerificationMode, VersionedTransaction,
         },
         transaction_context::{InstructionTrace, TransactionAccount, TransactionContext},
@@ -3439,10 +3439,7 @@ impl Bank {
     pub fn prepare_entry_batch(&self, txs: Vec<VersionedTransaction>) -> Result<TransactionBatch> {
         let sanitized_txs = txs
             .into_iter()
-            .map(|tx| {
+            .map(|tx| SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self))
-                let message_hash = tx.message.hash();
-                SanitizedTransaction::try_create(tx, message_hash, None, self)
-            })
             .collect::<Result<Vec<_>>>()?;
         let lock_results = self
             .rc

runtime/src/bank/address_lookup_table.rs

@@ -8,9 +8,9 @@ use {
     },
 };
 
-impl AddressLoader for Bank {
+impl AddressLoader for &Bank {
     fn load_addresses(
-        &self,
+        self,
         address_table_lookups: &[MessageAddressTableLookup],
     ) -> TransactionResult<LoadedAddresses> {
         if !self.versioned_tx_message_enabled() {

runtime/src/cost_tracker.rs

@@ -263,7 +263,7 @@ mod tests {
             hash::Hash,
             signature::{Keypair, Signer},
             system_transaction,
-            transaction::{DisabledAddressLoader, VersionedTransaction},
+            transaction::{MessageHash, SimpleAddressLoader, VersionedTransaction},
         },
         solana_vote_program::vote_transaction,
         std::{cmp, sync::Arc},
@@ -326,12 +326,11 @@ mod tests {
             &keypair,
             None,
         );
-        let message_hash = transaction.message.hash();
         let vote_transaction = SanitizedTransaction::try_create(
             VersionedTransaction::from(transaction),
-            message_hash,
+            MessageHash::Compute,
             Some(true),
-            &DisabledAddressLoader,
+            SimpleAddressLoader::Disabled,
         )
         .unwrap();
         (vote_transaction, vec![mint_keypair.pubkey()], 10)

sdk/src/transaction/sanitized.rs

@@ -39,14 +39,35 @@ pub struct TransactionAccountLocks<'a> {
     pub writable: Vec<&'a Pubkey>,
 }
 
-pub trait AddressLoader {
+pub trait AddressLoader: Clone {
-    fn load_addresses(&self, lookups: &[MessageAddressTableLookup]) -> Result<LoadedAddresses>;
+    fn load_addresses(self, lookups: &[MessageAddressTableLookup]) -> Result<LoadedAddresses>;
 }
 
-pub struct DisabledAddressLoader;
+#[derive(Clone)]
-impl AddressLoader for DisabledAddressLoader {
+pub enum SimpleAddressLoader {
-    fn load_addresses(&self, _lookups: &[MessageAddressTableLookup]) -> Result<LoadedAddresses> {
+    Disabled,
-        Err(TransactionError::UnsupportedVersion)
+    Enabled(LoadedAddresses),
+}
+
+impl AddressLoader for SimpleAddressLoader {
+    fn load_addresses(self, _lookups: &[MessageAddressTableLookup]) -> Result<LoadedAddresses> {
+        match self {
+            Self::Disabled => Err(TransactionError::AddressLookupTableNotFound),
+            Self::Enabled(loaded_addresses) => Ok(loaded_addresses),
+        }
+    }
+}
+
+/// Type that represents whether the transaction message has been precomputed or
+/// not.
+pub enum MessageHash {
+    Precomputed(Hash),
+    Compute,
+}
+
+impl From<Hash> for MessageHash {
+    fn from(hash: Hash) -> Self {
+        Self::Precomputed(hash)
     }
 }
 
@@ -56,12 +77,17 @@ impl SanitizedTransaction {
     /// the address for each table index.
     pub fn try_create(
         tx: VersionedTransaction,
-        message_hash: Hash,
+        message_hash: impl Into<MessageHash>,
         is_simple_vote_tx: Option<bool>,
-        address_loader: &impl AddressLoader,
+        address_loader: impl AddressLoader,
     ) -> Result<Self> {
         tx.sanitize()?;
 
+        let message_hash = match message_hash.into() {
+            MessageHash::Compute => tx.message.hash(),
+            MessageHash::Precomputed(hash) => hash,
+        };
+
         let signatures = tx.signatures;
         let message = match tx.message {
             VersionedMessage::Legacy(message) => SanitizedMessage::Legacy(message),