Proposal: Use SanitizedMessage when possible in `svm/account_loader.rs` (#35390)

This commit is contained in:
Lucas Steuernagel 2024-03-02 06:12:24 -03:00 committed by GitHub
parent f31ec1f2d6
commit bd93285025
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 27 additions and 58 deletions

View File

@ -7538,13 +7538,13 @@ impl TransactionProcessingCallback for Bank {
fn check_account_access( fn check_account_access(
&self, &self,
tx: &SanitizedTransaction, message: &SanitizedMessage,
account_index: usize, account_index: usize,
account: &AccountSharedData, account: &AccountSharedData,
error_counters: &mut TransactionErrorMetrics, error_counters: &mut TransactionErrorMetrics,
) -> Result<()> { ) -> Result<()> {
if self.get_reward_interval() == RewardInterval::InsideInterval if self.get_reward_interval() == RewardInterval::InsideInterval
&& tx.message().is_writable(account_index) && message.is_writable(account_index)
&& solana_stake_program::check_id(account.owner()) && solana_stake_program::check_id(account.owner())
{ {
error_counters.program_execution_temporarily_restricted += 1; error_counters.program_execution_temporarily_restricted += 1;

View File

@ -66,15 +66,14 @@ pub fn load_accounts<CB: TransactionProcessingCallback>(
.zip(lock_results) .zip(lock_results)
.map(|etx| match etx { .map(|etx| match etx {
(tx, (Ok(()), nonce, lamports_per_signature)) => { (tx, (Ok(()), nonce, lamports_per_signature)) => {
let message = tx.message();
let fee = if let Some(lamports_per_signature) = lamports_per_signature { let fee = if let Some(lamports_per_signature) = lamports_per_signature {
fee_structure.calculate_fee( fee_structure.calculate_fee(
tx.message(), message,
*lamports_per_signature, *lamports_per_signature,
&process_compute_budget_instructions( &process_compute_budget_instructions(message.program_instructions_iter())
tx.message().program_instructions_iter(), .unwrap_or_default()
) .into(),
.unwrap_or_default()
.into(),
feature_set feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()), .is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
feature_set.is_active(&remove_rounding_in_fee_calculation::id()), feature_set.is_active(&remove_rounding_in_fee_calculation::id()),
@ -86,7 +85,7 @@ pub fn load_accounts<CB: TransactionProcessingCallback>(
// load transactions // load transactions
let loaded_transaction = match load_transaction_accounts( let loaded_transaction = match load_transaction_accounts(
callbacks, callbacks,
tx, message,
fee, fee,
error_counters, error_counters,
account_overrides, account_overrides,
@ -101,7 +100,7 @@ pub fn load_accounts<CB: TransactionProcessingCallback>(
let nonce = if let Some(nonce) = nonce { let nonce = if let Some(nonce) = nonce {
match NonceFull::from_partial( match NonceFull::from_partial(
nonce, nonce,
tx.message(), message,
&loaded_transaction.accounts, &loaded_transaction.accounts,
&loaded_transaction.rent_debits, &loaded_transaction.rent_debits,
) { ) {
@ -123,25 +122,19 @@ pub fn load_accounts<CB: TransactionProcessingCallback>(
fn load_transaction_accounts<CB: TransactionProcessingCallback>( fn load_transaction_accounts<CB: TransactionProcessingCallback>(
callbacks: &CB, callbacks: &CB,
tx: &SanitizedTransaction, message: &SanitizedMessage,
fee: u64, fee: u64,
error_counters: &mut TransactionErrorMetrics, error_counters: &mut TransactionErrorMetrics,
account_overrides: Option<&AccountOverrides>, account_overrides: Option<&AccountOverrides>,
program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>, program_accounts: &HashMap<Pubkey, (&Pubkey, u64)>,
loaded_programs: &LoadedProgramsForTxBatch, loaded_programs: &LoadedProgramsForTxBatch,
) -> Result<LoadedTransaction> { ) -> Result<LoadedTransaction> {
// NOTE: this check will never fail because `tx` is sanitized
if tx.signatures().is_empty() && fee != 0 {
return Err(TransactionError::MissingSignatureForFee);
}
let feature_set = callbacks.get_feature_set(); let feature_set = callbacks.get_feature_set();
// There is no way to predict what program will execute without an error // There is no way to predict what program will execute without an error
// If a fee can pay for execution then the program will be scheduled // If a fee can pay for execution then the program will be scheduled
let mut validated_fee_payer = false; let mut validated_fee_payer = false;
let mut tx_rent: TransactionRent = 0; let mut tx_rent: TransactionRent = 0;
let message = tx.message();
let account_keys = message.account_keys(); let account_keys = message.account_keys();
let mut accounts_found = Vec::with_capacity(account_keys.len()); let mut accounts_found = Vec::with_capacity(account_keys.len());
let mut account_deps = Vec::with_capacity(account_keys.len()); let mut account_deps = Vec::with_capacity(account_keys.len());
@ -149,7 +142,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
let rent_collector = callbacks.get_rent_collector(); let rent_collector = callbacks.get_rent_collector();
let requested_loaded_accounts_data_size_limit = let requested_loaded_accounts_data_size_limit =
get_requested_loaded_accounts_data_size_limit(tx)?; get_requested_loaded_accounts_data_size_limit(message)?;
let mut accumulated_accounts_data_size: usize = 0; let mut accumulated_accounts_data_size: usize = 0;
let instruction_accounts = message let instruction_accounts = message
@ -231,7 +224,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
if !validated_fee_payer && message.is_non_loader_key(i) { if !validated_fee_payer && message.is_non_loader_key(i) {
if i != 0 { if i != 0 {
warn!("Payer index should be 0! {:?}", tx); warn!("Payer index should be 0! {:?}", message);
} }
validate_fee_payer( validate_fee_payer(
@ -246,7 +239,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
validated_fee_payer = true; validated_fee_payer = true;
} }
callbacks.check_account_access(tx, i, &account, error_counters)?; callbacks.check_account_access(message, i, &account, error_counters)?;
tx_rent += rent; tx_rent += rent;
rent_debits.insert(key, rent, account.lamports()); rent_debits.insert(key, rent, account.lamports());
@ -351,10 +344,10 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
/// user requested loaded accounts size. /// user requested loaded accounts size.
/// Note, requesting zero bytes will result transaction error /// Note, requesting zero bytes will result transaction error
fn get_requested_loaded_accounts_data_size_limit( fn get_requested_loaded_accounts_data_size_limit(
tx: &SanitizedTransaction, sanitized_message: &SanitizedMessage,
) -> Result<Option<NonZeroUsize>> { ) -> Result<Option<NonZeroUsize>> {
let compute_budget_limits = let compute_budget_limits =
process_compute_budget_instructions(tx.message().program_instructions_iter()) process_compute_budget_instructions(sanitized_message.program_instructions_iter())
.unwrap_or_default(); .unwrap_or_default();
// sanitize against setting size limit to zero // sanitize against setting size limit to zero
NonZeroUsize::new( NonZeroUsize::new(
@ -1149,7 +1142,7 @@ mod tests {
)); ));
assert_eq!( assert_eq!(
*expected_result, *expected_result,
get_requested_loaded_accounts_data_size_limit(&tx) get_requested_loaded_accounts_data_size_limit(tx.message())
); );
} }
@ -1429,30 +1422,6 @@ mod tests {
assert_eq!(result.unwrap(), expected); assert_eq!(result.unwrap(), expected);
} }
#[test]
fn test_load_transaction_accounts_failure() {
let message = Message::default();
let legacy = LegacyMessage::new(message);
let sanitized_message = SanitizedMessage::Legacy(legacy);
let mock_bank = TestCallbacks::default();
let mut error_counter = TransactionErrorMetrics::default();
let loaded_programs = LoadedProgramsForTxBatch::default();
let sanitized_transaction =
SanitizedTransaction::new_for_tests(sanitized_message, vec![], false);
let result = load_transaction_accounts(
&mock_bank,
&sanitized_transaction,
32,
&mut error_counter,
None,
&HashMap::new(),
&loaded_programs,
);
assert_eq!(result.err(), Some(TransactionError::MissingSignatureForFee));
}
#[test] #[test]
fn test_load_transaction_accounts_fail_to_validate_fee_payer() { fn test_load_transaction_accounts_fail_to_validate_fee_payer() {
let message = Message { let message = Message {
@ -1479,7 +1448,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1524,7 +1493,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1591,7 +1560,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1635,7 +1604,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1679,7 +1648,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1730,7 +1699,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1799,7 +1768,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1857,7 +1826,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -1920,7 +1889,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,
@ -2009,7 +1978,7 @@ mod tests {
); );
let result = load_transaction_accounts( let result = load_transaction_accounts(
&mock_bank, &mock_bank,
&sanitized_transaction, sanitized_transaction.message(),
32, 32,
&mut error_counter, &mut error_counter,
None, None,

View File

@ -78,7 +78,7 @@ pub trait TransactionProcessingCallback {
fn check_account_access( fn check_account_access(
&self, &self,
_tx: &SanitizedTransaction, _message: &SanitizedMessage,
_account_index: usize, _account_index: usize,
_account: &AccountSharedData, _account: &AccountSharedData,
_error_counters: &mut TransactionErrorMetrics, _error_counters: &mut TransactionErrorMetrics,