From d0f3f441b6b1e97952a819a4f67e6309d4d78f28 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Thu, 25 Jan 2024 10:12:24 +0800 Subject: [PATCH 1/2] Simplify and refactor tx message creation in tests --- program-runtime/src/message_processor.rs | 120 ++++++++++---------- rpc/src/rpc.rs | 2 +- runtime/benches/prioritization_fee_cache.rs | 2 +- runtime/src/bank/tests.rs | 72 ++++++------ runtime/src/compute_budget_details.rs | 9 +- runtime/src/prioritization_fee_cache.rs | 2 +- sdk/program/src/sysvar/instructions.rs | 32 +++--- svm/tests/rent_state.rs | 2 +- 8 files changed, 120 insertions(+), 121 deletions(-) diff --git a/program-runtime/src/message_processor.rs b/program-runtime/src/message_processor.rs index a428cf930efeca..741b7668d757b1 100644 --- a/program-runtime/src/message_processor.rs +++ b/program-runtime/src/message_processor.rs @@ -180,13 +180,14 @@ mod tests { solana_sdk::{ account::{AccountSharedData, ReadableAccount}, instruction::{AccountMeta, Instruction, InstructionError}, - message::{AccountKeys, LegacyMessage, Message}, + message::{AccountKeys, Message}, native_loader::{self, create_loadable_account_for_test}, pubkey::Pubkey, rent::Rent, secp256k1_instruction::new_secp256k1_instruction, - secp256k1_program, + secp256k1_program, system_program, }, + std::convert::TryFrom, }; #[derive(Debug, Serialize, Deserialize)] @@ -198,6 +199,10 @@ mod tests { ModifyReadonly, } + fn new_sanitized_message(message: Message) -> SanitizedMessage { + SanitizedMessage::try_from(message).unwrap() + } + #[test] fn test_process_message_readonly_handling() { #[derive(Serialize, Deserialize)] @@ -272,21 +277,20 @@ mod tests { AccountMeta::new_readonly(readonly_pubkey, false), ]; - let message = - SanitizedMessage::Legacy(LegacyMessage::new(Message::new_with_compiled_instructions( - 1, - 0, - 2, - account_keys.clone(), - Hash::default(), - AccountKeys::new(&account_keys, None).compile_instructions(&[ - Instruction::new_with_bincode( - mock_system_program_id, - &MockSystemInstruction::Correct, - account_metas.clone(), - ), - ]), - ))); + let message = new_sanitized_message(Message::new_with_compiled_instructions( + 1, + 0, + 2, + account_keys.clone(), + Hash::default(), + AccountKeys::new(&account_keys, None).compile_instructions(&[ + Instruction::new_with_bincode( + mock_system_program_id, + &MockSystemInstruction::Correct, + account_metas.clone(), + ), + ]), + )); let sysvar_cache = SysvarCache::default(); let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( @@ -322,21 +326,20 @@ mod tests { 0 ); - let message = - SanitizedMessage::Legacy(LegacyMessage::new(Message::new_with_compiled_instructions( - 1, - 0, - 2, - account_keys.clone(), - Hash::default(), - AccountKeys::new(&account_keys, None).compile_instructions(&[ - Instruction::new_with_bincode( - mock_system_program_id, - &MockSystemInstruction::TransferLamports { lamports: 50 }, - account_metas.clone(), - ), - ]), - ))); + let message = new_sanitized_message(Message::new_with_compiled_instructions( + 1, + 0, + 2, + account_keys.clone(), + Hash::default(), + AccountKeys::new(&account_keys, None).compile_instructions(&[ + Instruction::new_with_bincode( + mock_system_program_id, + &MockSystemInstruction::TransferLamports { lamports: 50 }, + account_metas.clone(), + ), + ]), + )); let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( &message, @@ -361,21 +364,20 @@ mod tests { )) ); - let message = - SanitizedMessage::Legacy(LegacyMessage::new(Message::new_with_compiled_instructions( - 1, - 0, - 2, - account_keys.clone(), - Hash::default(), - AccountKeys::new(&account_keys, None).compile_instructions(&[ - Instruction::new_with_bincode( - mock_system_program_id, - &MockSystemInstruction::ChangeData { data: 50 }, - account_metas, - ), - ]), - ))); + let message = new_sanitized_message(Message::new_with_compiled_instructions( + 1, + 0, + 2, + account_keys.clone(), + Hash::default(), + AccountKeys::new(&account_keys, None).compile_instructions(&[ + Instruction::new_with_bincode( + mock_system_program_id, + &MockSystemInstruction::ChangeData { data: 50 }, + account_metas, + ), + ]), + )); let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( &message, @@ -496,14 +498,14 @@ mod tests { ]; // Try to borrow mut the same account - let message = SanitizedMessage::Legacy(LegacyMessage::new(Message::new( + let message = new_sanitized_message(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::BorrowFail, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0).unwrap()), - ))); + )); let sysvar_cache = SysvarCache::default(); let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( @@ -530,14 +532,14 @@ mod tests { ); // Try to borrow mut the same account in a safe way - let message = SanitizedMessage::Legacy(LegacyMessage::new(Message::new( + let message = new_sanitized_message(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::MultiBorrowMut, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0).unwrap()), - ))); + )); let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( &message, @@ -557,7 +559,7 @@ mod tests { assert!(result.is_ok()); // Do work on the same transaction account but at different instruction accounts - let message = SanitizedMessage::Legacy(LegacyMessage::new(Message::new( + let message = new_sanitized_message(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::DoWork { @@ -567,7 +569,7 @@ mod tests { account_metas, )], Some(transaction_context.get_key_of_account_at_index(0).unwrap()), - ))); + )); let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( &message, @@ -623,6 +625,10 @@ mod tests { let mut mock_program_account = AccountSharedData::new(1, 0, &native_loader::id()); mock_program_account.set_executable(true); let accounts = vec![ + ( + Pubkey::new_unique(), + AccountSharedData::new(1, 0, &system_program::id()), + ), (secp256k1_program::id(), secp256k1_account), (mock_program_id, mock_program_account), ]; @@ -642,13 +648,13 @@ mod tests { } } }; - let message = SanitizedMessage::Legacy(LegacyMessage::new(Message::new( + let message = new_sanitized_message(Message::new( &[ new_secp256k1_instruction(&secret_key, b"hello"), Instruction::new_with_bytes(mock_program_id, &[], vec![]), ], - None, - ))); + Some(transaction_context.get_key_of_account_at_index(0).unwrap()), + )); let sysvar_cache = SysvarCache::default(); let mut programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default(); programs_loaded_for_tx_batch.replenish( @@ -658,7 +664,7 @@ mod tests { let mut programs_modified_by_tx = LoadedProgramsForTxBatch::default(); let result = MessageProcessor::process_message( &message, - &[vec![0], vec![1]], + &[vec![1], vec![2]], &mut transaction_context, None, &programs_loaded_for_tx_batch, diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 7bde6b837f2a13..caeb0953109fbb 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -5070,7 +5070,7 @@ pub mod tests { let prioritization_fee_cache = &self.meta.prioritization_fee_cache; let transactions: Vec<_> = transactions .into_iter() - .map(|tx| SanitizedTransaction::try_from_legacy_transaction(tx).unwrap()) + .map(SanitizedTransaction::from_transaction_for_tests) .collect(); prioritization_fee_cache.update(&bank, transactions.iter()); } diff --git a/runtime/benches/prioritization_fee_cache.rs b/runtime/benches/prioritization_fee_cache.rs index 506aac4fb729a3..8c6bf1fe0a7d68 100644 --- a/runtime/benches/prioritization_fee_cache.rs +++ b/runtime/benches/prioritization_fee_cache.rs @@ -36,7 +36,7 @@ fn build_sanitized_transaction( Some(signer_account), )); - SanitizedTransaction::try_from_legacy_transaction(transaction).unwrap() + SanitizedTransaction::from_transaction_for_tests(transaction) } #[bench] diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 02bb7f5c08a0de..765a19d9a522d9 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -195,6 +195,10 @@ fn create_genesis_config(lamports: u64) -> (GenesisConfig, Keypair) { solana_sdk::genesis_config::create_genesis_config(lamports) } +fn new_sanitized_message(message: Message) -> SanitizedMessage { + SanitizedMessage::try_from(message).unwrap() +} + #[test] fn test_race_register_tick_freeze() { solana_logger::setup(); @@ -2666,7 +2670,7 @@ fn test_bank_tx_compute_unit_fee() { let (bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); let expected_fee_paid = calculate_test_fee( - &SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(), + &new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))), genesis_config .fee_rate_governor .create_fee_calculator() @@ -2794,7 +2798,7 @@ fn test_bank_blockhash_fee_structure() { assert_eq!(bank.process_transaction(&tx), Ok(())); assert_eq!(bank.get_balance(&key), 1); let cheap_fee = calculate_test_fee( - &SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(), + &new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))), cheap_lamports_per_signature, &bank.fee_structure, ); @@ -2810,7 +2814,7 @@ fn test_bank_blockhash_fee_structure() { assert_eq!(bank.process_transaction(&tx), Ok(())); assert_eq!(bank.get_balance(&key), 1); let expensive_fee = calculate_test_fee( - &SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(), + &new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))), expensive_lamports_per_signature, &bank.fee_structure, ); @@ -2856,7 +2860,7 @@ fn test_bank_blockhash_compute_unit_fee_structure() { assert_eq!(bank.process_transaction(&tx), Ok(())); assert_eq!(bank.get_balance(&key), 1); let cheap_fee = calculate_test_fee( - &SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(), + &new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))), cheap_lamports_per_signature, &bank.fee_structure, ); @@ -2872,7 +2876,7 @@ fn test_bank_blockhash_compute_unit_fee_structure() { assert_eq!(bank.process_transaction(&tx), Ok(())); assert_eq!(bank.get_balance(&key), 1); let expensive_fee = calculate_test_fee( - &SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(), + &new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))), expensive_lamports_per_signature, &bank.fee_structure, ); @@ -2979,8 +2983,7 @@ fn test_filter_program_errors_and_collect_compute_unit_fee() { .fee_rate_governor .burn( calculate_test_fee( - &SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))) - .unwrap(), + &new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))), genesis_config .fee_rate_governor .create_fee_calculator() @@ -5275,7 +5278,7 @@ fn test_nonce_transaction() { recent_message.recent_blockhash = bank.last_blockhash(); let mut expected_balance = 4_650_000 - bank - .get_fee_for_message(&recent_message.try_into().unwrap()) + .get_fee_for_message(&new_sanitized_message(recent_message)) .unwrap(); assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance); assert_eq!(bank.get_balance(&nonce_pubkey), 250_000); @@ -5334,7 +5337,7 @@ fn test_nonce_transaction() { let mut recent_message = nonce_tx.message.clone(); recent_message.recent_blockhash = bank.last_blockhash(); expected_balance -= bank - .get_fee_for_message(&SanitizedMessage::try_from(recent_message).unwrap()) + .get_fee_for_message(&new_sanitized_message(recent_message)) .unwrap(); assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance); assert_ne!( @@ -5402,7 +5405,7 @@ fn test_nonce_transaction_with_tx_wide_caps() { recent_message.recent_blockhash = bank.last_blockhash(); let mut expected_balance = 4_650_000 - bank - .get_fee_for_message(&recent_message.try_into().unwrap()) + .get_fee_for_message(&new_sanitized_message(recent_message)) .unwrap(); assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance); assert_eq!(bank.get_balance(&nonce_pubkey), 250_000); @@ -5461,7 +5464,7 @@ fn test_nonce_transaction_with_tx_wide_caps() { let mut recent_message = nonce_tx.message.clone(); recent_message.recent_blockhash = bank.last_blockhash(); expected_balance -= bank - .get_fee_for_message(&SanitizedMessage::try_from(recent_message).unwrap()) + .get_fee_for_message(&new_sanitized_message(recent_message)) .unwrap(); assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance); assert_ne!( @@ -5593,7 +5596,7 @@ fn test_nonce_payer() { bank.get_balance(&nonce_pubkey), nonce_starting_balance - bank - .get_fee_for_message(&recent_message.try_into().unwrap()) + .get_fee_for_message(&new_sanitized_message(recent_message)) .unwrap() ); assert_ne!( @@ -5660,7 +5663,7 @@ fn test_nonce_payer_tx_wide_cap() { bank.get_balance(&nonce_pubkey), nonce_starting_balance - bank - .get_fee_for_message(&recent_message.try_into().unwrap()) + .get_fee_for_message(&new_sanitized_message(recent_message)) .unwrap() ); assert_ne!( @@ -10034,8 +10037,7 @@ fn calculate_test_fee( #[test] fn test_calculate_fee() { // Default: no fee. - let message = - SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(); + let message = new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))); assert_eq!( calculate_test_fee( &message, @@ -10066,7 +10068,7 @@ fn test_calculate_fee() { let key1 = Pubkey::new_unique(); let ix0 = system_instruction::transfer(&key0, &key1, 1); let ix1 = system_instruction::transfer(&key1, &key0, 1); - let message = SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&key0))).unwrap(); + let message = new_sanitized_message(Message::new(&[ix0, ix1], Some(&key0))); assert_eq!( calculate_test_fee( &message, @@ -10091,8 +10093,7 @@ fn test_calculate_fee_compute_units() { // One signature, no unit request - let message = - SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(); + let message = new_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))); assert_eq!( calculate_test_fee(&message, 1, &fee_structure,), max_fee + lamports_per_signature @@ -10102,8 +10103,7 @@ fn test_calculate_fee_compute_units() { let ix0 = system_instruction::transfer(&Pubkey::new_unique(), &Pubkey::new_unique(), 1); let ix1 = system_instruction::transfer(&Pubkey::new_unique(), &Pubkey::new_unique(), 1); - let message = - SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&Pubkey::new_unique()))).unwrap(); + let message = new_sanitized_message(Message::new(&[ix0, ix1], Some(&Pubkey::new_unique()))); assert_eq!( calculate_test_fee(&message, 1, &fee_structure,), max_fee + 3 * lamports_per_signature @@ -10129,15 +10129,14 @@ fn test_calculate_fee_compute_units() { PrioritizationFeeType::ComputeUnitPrice(PRIORITIZATION_FEE_RATE), requested_compute_units as u64, ); - let message = SanitizedMessage::try_from(Message::new( + let message = new_sanitized_message(Message::new( &[ ComputeBudgetInstruction::set_compute_unit_limit(requested_compute_units), ComputeBudgetInstruction::set_compute_unit_price(PRIORITIZATION_FEE_RATE), Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]), ], Some(&Pubkey::new_unique()), - )) - .unwrap(); + )); let fee = calculate_test_fee(&message, 1, &fee_structure); assert_eq!( fee, @@ -10161,14 +10160,13 @@ fn test_calculate_prioritization_fee() { ); let prioritization_fee = prioritization_fee_details.get_fee(); - let message = SanitizedMessage::try_from(Message::new( + let message = new_sanitized_message(Message::new( &[ ComputeBudgetInstruction::set_compute_unit_limit(request_units), ComputeBudgetInstruction::set_compute_unit_price(request_unit_price), ], Some(&Pubkey::new_unique()), - )) - .unwrap(); + )); let fee = calculate_test_fee( &message, @@ -10202,24 +10200,22 @@ fn test_calculate_fee_secp256k1() { data: vec![1], }; - let message = SanitizedMessage::try_from(Message::new( + let message = new_sanitized_message(Message::new( &[ ix0.clone(), secp_instruction1.clone(), secp_instruction2.clone(), ], Some(&key0), - )) - .unwrap(); + )); assert_eq!(calculate_test_fee(&message, 1, &fee_structure,), 2); secp_instruction1.data = vec![0]; secp_instruction2.data = vec![10]; - let message = SanitizedMessage::try_from(Message::new( + let message = new_sanitized_message(Message::new( &[ix0, secp_instruction1, secp_instruction2], Some(&key0), - )) - .unwrap(); + )); assert_eq!(calculate_test_fee(&message, 1, &fee_structure,), 11); } @@ -10745,7 +10741,7 @@ fn test_invalid_rent_state_changes_fee_payer() { .unwrap(); // Dummy message to determine fee amount - let dummy_message = SanitizedMessage::try_from(Message::new_with_blockhash( + let dummy_message = new_sanitized_message(Message::new_with_blockhash( &[system_instruction::transfer( &rent_exempt_fee_payer.pubkey(), &recipient, @@ -10753,8 +10749,7 @@ fn test_invalid_rent_state_changes_fee_payer() { )], Some(&rent_exempt_fee_payer.pubkey()), &recent_blockhash, - )) - .unwrap(); + )); let fee = bank.get_fee_for_message(&dummy_message).unwrap(); // RentPaying fee-payer can remain RentPaying @@ -11814,7 +11809,7 @@ fn test_calculate_fee_with_congestion_multiplier() { let key1 = Pubkey::new_unique(); let ix0 = system_instruction::transfer(&key0, &key1, 1); let ix1 = system_instruction::transfer(&key1, &key0, 1); - let message = SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&key0))).unwrap(); + let message = new_sanitized_message(Message::new(&[ix0, ix1], Some(&key0))); // assert when lamports_per_signature is less than BASE_LAMPORTS, turnning on/off // congestion_multiplier has no effect on fee. @@ -11843,7 +11838,7 @@ fn test_calculate_fee_with_request_heap_frame_flag() { lamports_per_signature: signature_fee, ..FeeStructure::default() }; - let message = SanitizedMessage::try_from(Message::new( + let message = new_sanitized_message(Message::new( &[ system_instruction::transfer(&key0, &key1, 1), ComputeBudgetInstruction::set_compute_unit_limit(request_cu as u32), @@ -11851,8 +11846,7 @@ fn test_calculate_fee_with_request_heap_frame_flag() { ComputeBudgetInstruction::set_compute_unit_price(lamports_per_cu * 1_000_000), ], Some(&key0), - )) - .unwrap(); + )); // assert when request_heap_frame is presented in tx, prioritization fee will be counted // into transaction fee diff --git a/runtime/src/compute_budget_details.rs b/runtime/src/compute_budget_details.rs index 69756d4567ff70..72b10a11b33bcc 100644 --- a/runtime/src/compute_budget_details.rs +++ b/runtime/src/compute_budget_details.rs @@ -95,8 +95,7 @@ mod tests { ); // assert for SanitizedTransaction - let sanitized_transaction = - SanitizedTransaction::try_from_legacy_transaction(transaction).unwrap(); + let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(transaction); assert_eq!( sanitized_transaction.get_compute_budget_details(false), Some(ComputeBudgetDetails { @@ -133,8 +132,7 @@ mod tests { ); // assert for SanitizedTransaction - let sanitized_transaction = - SanitizedTransaction::try_from_legacy_transaction(transaction).unwrap(); + let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(transaction); assert_eq!( sanitized_transaction.get_compute_budget_details(false), Some(ComputeBudgetDetails { @@ -171,8 +169,7 @@ mod tests { ); // assert for SanitizedTransaction - let sanitized_transaction = - SanitizedTransaction::try_from_legacy_transaction(transaction).unwrap(); + let sanitized_transaction = SanitizedTransaction::from_transaction_for_tests(transaction); assert_eq!( sanitized_transaction.get_compute_budget_details(false), Some(ComputeBudgetDetails { diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index 839519020ff42f..0490f594451b9c 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -459,7 +459,7 @@ mod tests { Some(signer_account), )); - SanitizedTransaction::try_from_legacy_transaction(transaction).unwrap() + SanitizedTransaction::from_transaction_for_tests(transaction) } // update fee cache is asynchronous, this test helper blocks until update is completed. diff --git a/sdk/program/src/sysvar/instructions.rs b/sdk/program/src/sysvar/instructions.rs index a5a31735795832..d8b210a8d97bb4 100644 --- a/sdk/program/src/sysvar/instructions.rs +++ b/sdk/program/src/sysvar/instructions.rs @@ -305,6 +305,10 @@ mod tests { std::convert::TryFrom, }; + fn new_sanitized_message(message: LegacyMessage) -> SanitizedMessage { + SanitizedMessage::try_from(message).unwrap() + } + #[test] fn test_load_store_instruction() { let mut data = [4u8; 10]; @@ -327,11 +331,11 @@ mod tests { &0, vec![AccountMeta::new(Pubkey::new_unique(), false)], ); - let sanitized_message = SanitizedMessage::try_from(LegacyMessage::new( + let message = LegacyMessage::new( &[instruction0.clone(), instruction1.clone()], Some(&Pubkey::new_unique()), - )) - .unwrap(); + ); + let sanitized_message = new_sanitized_message(message); let key = id(); let mut lamports = 0; @@ -381,11 +385,9 @@ mod tests { &0, vec![AccountMeta::new(Pubkey::new_unique(), false)], ); - let sanitized_message = SanitizedMessage::try_from(LegacyMessage::new( - &[instruction0, instruction1], - Some(&Pubkey::new_unique()), - )) - .unwrap(); + let message = + LegacyMessage::new(&[instruction0, instruction1], Some(&Pubkey::new_unique())); + let sanitized_message = new_sanitized_message(message); let key = id(); let mut lamports = 0; @@ -435,15 +437,15 @@ mod tests { &0, vec![AccountMeta::new(Pubkey::new_unique(), false)], ); - let sanitized_message = SanitizedMessage::try_from(LegacyMessage::new( + let message = LegacyMessage::new( &[ instruction0.clone(), instruction1.clone(), instruction2.clone(), ], Some(&Pubkey::new_unique()), - )) - .unwrap(); + ); + let sanitized_message = new_sanitized_message(message); let key = id(); let mut lamports = 0; @@ -538,7 +540,7 @@ mod tests { ]; let message = LegacyMessage::new(&instructions, Some(&id1)); - let sanitized_message = SanitizedMessage::try_from(message).unwrap(); + let sanitized_message = new_sanitized_message(message); let serialized = serialize_instructions(&sanitized_message.decompile_instructions()); // assert that deserialize_instruction is compatible with SanitizedMessage::serialize_instructions @@ -560,9 +562,9 @@ mod tests { Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id1, true)]), ]; - let message = - SanitizedMessage::try_from(LegacyMessage::new(&instructions, Some(&id1))).unwrap(); - let serialized = serialize_instructions(&message.decompile_instructions()); + let message = LegacyMessage::new(&instructions, Some(&id1)); + let sanitized_message = new_sanitized_message(message); + let serialized = serialize_instructions(&sanitized_message.decompile_instructions()); assert_eq!( deserialize_instruction(instructions.len(), &serialized).unwrap_err(), SanitizeError::IndexOutOfBounds, diff --git a/svm/tests/rent_state.rs b/svm/tests/rent_state.rs index d24a32ac352fbf..f3ea728f6b874f 100644 --- a/svm/tests/rent_state.rs +++ b/svm/tests/rent_state.rs @@ -55,7 +55,7 @@ fn test_rent_state_list_len() { last_block_hash, ); let num_accounts = tx.message().account_keys.len(); - let sanitized_tx = SanitizedTransaction::try_from_legacy_transaction(tx).unwrap(); + let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); let mut error_counters = TransactionErrorMetrics::default(); let loaded_txs = load_accounts( &bank, From d58b00c0cd853ac6c81eb5ad2cce9ee33325a0b6 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Thu, 25 Jan 2024 10:36:48 +0800 Subject: [PATCH 2/2] Rename SanitizedMessage::try_from to try_from_legacy_message --- banks-server/src/banks_server.rs | 3 +- program-runtime/src/message_processor.rs | 3 +- programs/sbf/tests/programs.rs | 6 +-- runtime/src/bank/tests.rs | 4 +- runtime/src/bank_client.rs | 3 +- sdk/benches/serialize_instructions.rs | 25 ++++++---- sdk/program/src/message/sanitized.rs | 58 ++++++++++++------------ sdk/program/src/sysvar/instructions.rs | 3 +- sdk/src/nonce_info.rs | 2 +- svm/src/account_loader.rs | 4 +- 10 files changed, 57 insertions(+), 54 deletions(-) diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 22f63e9f60a0d5..b3028c0132ed48 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -31,7 +31,6 @@ use { }, solana_svm::transaction_results::TransactionExecutionResult, std::{ - convert::TryFrom, io, net::{Ipv4Addr, SocketAddr}, sync::{atomic::AtomicBool, Arc, RwLock}, @@ -418,7 +417,7 @@ impl Banks for BanksServer { commitment: CommitmentLevel, ) -> Option { let bank = self.bank(commitment); - let sanitized_message = SanitizedMessage::try_from(message).ok()?; + let sanitized_message = SanitizedMessage::try_from_legacy_message(message).ok()?; bank.get_fee_for_message(&sanitized_message) } } diff --git a/program-runtime/src/message_processor.rs b/program-runtime/src/message_processor.rs index 741b7668d757b1..507197298479d9 100644 --- a/program-runtime/src/message_processor.rs +++ b/program-runtime/src/message_processor.rs @@ -187,7 +187,6 @@ mod tests { secp256k1_instruction::new_secp256k1_instruction, secp256k1_program, system_program, }, - std::convert::TryFrom, }; #[derive(Debug, Serialize, Deserialize)] @@ -200,7 +199,7 @@ mod tests { } fn new_sanitized_message(message: Message) -> SanitizedMessage { - SanitizedMessage::try_from(message).unwrap() + SanitizedMessage::try_from_legacy_message(message).unwrap() } #[test] diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index b29d78422dca51..dc4867ce7e40fd 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -200,7 +200,7 @@ fn execute_transactions( } .expect("lamports_per_signature must be available"); let fee = bank.get_fee_for_message_with_lamports_per_signature( - &SanitizedMessage::try_from(tx.message().clone()).unwrap(), + &SanitizedMessage::try_from_legacy_message(tx.message().clone()).unwrap(), lamports_per_signature, ); @@ -3705,7 +3705,7 @@ fn test_program_fees() { Some(&mint_keypair.pubkey()), ); - let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap(); + let sanitized_message = SanitizedMessage::try_from_legacy_message(message.clone()).unwrap(); let expected_normal_fee = fee_structure.calculate_fee( &sanitized_message, congestion_multiplier, @@ -3729,7 +3729,7 @@ fn test_program_fees() { ], Some(&mint_keypair.pubkey()), ); - let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap(); + let sanitized_message = SanitizedMessage::try_from_legacy_message(message.clone()).unwrap(); let expected_prioritized_fee = fee_structure.calculate_fee( &sanitized_message, congestion_multiplier, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 765a19d9a522d9..2283899d3ca30d 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -117,7 +117,7 @@ use { }, std::{ collections::{HashMap, HashSet}, - convert::{TryFrom, TryInto}, + convert::TryInto, fs::File, io::Read, str::FromStr, @@ -196,7 +196,7 @@ fn create_genesis_config(lamports: u64) -> (GenesisConfig, Keypair) { } fn new_sanitized_message(message: Message) -> SanitizedMessage { - SanitizedMessage::try_from(message).unwrap() + SanitizedMessage::try_from_legacy_message(message).unwrap() } #[test] diff --git a/runtime/src/bank_client.rs b/runtime/src/bank_client.rs index 7fe6418d4110b2..22a1631085870f 100644 --- a/runtime/src/bank_client.rs +++ b/runtime/src/bank_client.rs @@ -19,7 +19,6 @@ use { transport::{Result, TransportError}, }, std::{ - convert::TryFrom, io, sync::{Arc, Mutex}, thread::{sleep, Builder}, @@ -286,7 +285,7 @@ impl SyncClient for BankClient { } fn get_fee_for_message(&self, message: &Message) -> Result { - SanitizedMessage::try_from(message.clone()) + SanitizedMessage::try_from_legacy_message(message.clone()) .ok() .and_then(|sanitized_message| self.bank.get_fee_for_message(&sanitized_message)) .ok_or_else(|| { diff --git a/sdk/benches/serialize_instructions.rs b/sdk/benches/serialize_instructions.rs index 955bb948fca0d2..adf36497ec67d4 100644 --- a/sdk/benches/serialize_instructions.rs +++ b/sdk/benches/serialize_instructions.rs @@ -9,7 +9,6 @@ use { pubkey::{self, Pubkey}, sysvar::instructions::{self, construct_instructions_data}, }, - std::convert::TryFrom, test::Bencher, }; @@ -30,9 +29,11 @@ fn bench_bincode_instruction_serialize(b: &mut Bencher) { #[bench] fn bench_construct_instructions_data(b: &mut Bencher) { let instructions = make_instructions(); - let message = - SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) - .unwrap(); + let message = SanitizedMessage::try_from_legacy_message(Message::new( + &instructions, + Some(&Pubkey::new_unique()), + )) + .unwrap(); b.iter(|| { let instructions = message.decompile_instructions(); test::black_box(construct_instructions_data(&instructions)); @@ -51,9 +52,11 @@ fn bench_bincode_instruction_deserialize(b: &mut Bencher) { #[bench] fn bench_manual_instruction_deserialize(b: &mut Bencher) { let instructions = make_instructions(); - let message = - SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) - .unwrap(); + let message = SanitizedMessage::try_from_legacy_message(Message::new( + &instructions, + Some(&Pubkey::new_unique()), + )) + .unwrap(); let serialized = construct_instructions_data(&message.decompile_instructions()); b.iter(|| { for i in 0..instructions.len() { @@ -66,9 +69,11 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) { #[bench] fn bench_manual_instruction_deserialize_single(b: &mut Bencher) { let instructions = make_instructions(); - let message = - SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) - .unwrap(); + let message = SanitizedMessage::try_from_legacy_message(Message::new( + &instructions, + Some(&Pubkey::new_unique()), + )) + .unwrap(); let serialized = construct_instructions_data(&message.decompile_instructions()); b.iter(|| { #[allow(deprecated)] diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index 098a781ea4dbf7..d4c7638e136a72 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -98,14 +98,6 @@ impl From for SanitizeMessageError { } } -impl TryFrom for SanitizedMessage { - type Error = SanitizeMessageError; - fn try_from(message: legacy::Message) -> Result { - message.sanitize()?; - Ok(Self::Legacy(LegacyMessage::new(message))) - } -} - impl SanitizedMessage { /// Create a sanitized message from a sanitized versioned message. /// If the input message uses address tables, attempt to look up the @@ -126,6 +118,12 @@ impl SanitizedMessage { }) } + /// Create a sanitized legacy message + pub fn try_from_legacy_message(message: legacy::Message) -> Result { + message.sanitize()?; + Ok(Self::Legacy(LegacyMessage::new(message))) + } + /// Return true if this message contains duplicate account keys pub fn has_duplicates(&self) -> bool { match self { @@ -374,14 +372,14 @@ mod tests { use {super::*, crate::message::v0, std::collections::HashSet}; #[test] - fn test_try_from_message() { + fn test_try_from_legacy_message() { let legacy_message_with_no_signers = legacy::Message { account_keys: vec![Pubkey::new_unique()], ..legacy::Message::default() }; assert_eq!( - SanitizedMessage::try_from(legacy_message_with_no_signers).err(), + SanitizedMessage::try_from_legacy_message(legacy_message_with_no_signers).err(), Some(SanitizeMessageError::IndexOutOfBounds), ); } @@ -396,14 +394,16 @@ mod tests { CompiledInstruction::new(2, &(), vec![0, 1]), ]; - let message = SanitizedMessage::try_from(legacy::Message::new_with_compiled_instructions( - 1, - 0, - 2, - vec![key0, key1, loader_key], - Hash::default(), - instructions, - )) + let message = SanitizedMessage::try_from_legacy_message( + legacy::Message::new_with_compiled_instructions( + 1, + 0, + 2, + vec![key0, key1, loader_key], + Hash::default(), + instructions, + ), + ) .unwrap(); assert!(message.is_non_loader_key(0)); @@ -420,7 +420,7 @@ mod tests { let key4 = Pubkey::new_unique(); let key5 = Pubkey::new_unique(); - let legacy_message = SanitizedMessage::try_from(legacy::Message { + let legacy_message = SanitizedMessage::try_from_legacy_message(legacy::Message { header: MessageHeader { num_required_signatures: 2, num_readonly_signed_accounts: 1, @@ -464,14 +464,16 @@ mod tests { CompiledInstruction::new(3, &(), vec![0, 0]), ]; - let message = SanitizedMessage::try_from(legacy::Message::new_with_compiled_instructions( - 2, - 1, - 2, - vec![signer0, signer1, non_signer, loader_key], - Hash::default(), - instructions, - )) + let message = SanitizedMessage::try_from_legacy_message( + legacy::Message::new_with_compiled_instructions( + 2, + 1, + 2, + vec![signer0, signer1, non_signer, loader_key], + Hash::default(), + instructions, + ), + ) .unwrap(); assert_eq!( @@ -502,7 +504,7 @@ mod tests { let key4 = Pubkey::new_unique(); let key5 = Pubkey::new_unique(); - let legacy_message = SanitizedMessage::try_from(legacy::Message { + let legacy_message = SanitizedMessage::try_from_legacy_message(legacy::Message { header: MessageHeader { num_required_signatures: 2, num_readonly_signed_accounts: 1, diff --git a/sdk/program/src/sysvar/instructions.rs b/sdk/program/src/sysvar/instructions.rs index d8b210a8d97bb4..28d5674177b838 100644 --- a/sdk/program/src/sysvar/instructions.rs +++ b/sdk/program/src/sysvar/instructions.rs @@ -302,11 +302,10 @@ mod tests { message::{Message as LegacyMessage, SanitizedMessage}, pubkey::Pubkey, }, - std::convert::TryFrom, }; fn new_sanitized_message(message: LegacyMessage) -> SanitizedMessage { - SanitizedMessage::try_from(message).unwrap() + SanitizedMessage::try_from_legacy_message(message).unwrap() } #[test] diff --git a/sdk/src/nonce_info.rs b/sdk/src/nonce_info.rs index 585f9fa2e3a687..c29d3db6bdb944 100644 --- a/sdk/src/nonce_info.rs +++ b/sdk/src/nonce_info.rs @@ -133,7 +133,7 @@ mod tests { instructions: &[Instruction], payer: Option<&Pubkey>, ) -> SanitizedMessage { - Message::new(instructions, payer).try_into().unwrap() + SanitizedMessage::try_from_legacy_message(Message::new(instructions, payer)).unwrap() } #[test] diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 334ad7679561ee..003296f93859f9 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -678,7 +678,7 @@ mod tests { instructions, ); - let message = SanitizedMessage::try_from(tx.message().clone()).unwrap(); + let message = SanitizedMessage::try_from_legacy_message(tx.message().clone()).unwrap(); let fee = FeeStructure::default().calculate_fee( &message, lamports_per_signature, @@ -1207,7 +1207,7 @@ mod tests { Hash::default(), ); - let message = SanitizedMessage::try_from(tx.message().clone()).unwrap(); + let message = SanitizedMessage::try_from_legacy_message(tx.message().clone()).unwrap(); let fee = FeeStructure::default().calculate_fee( &message, lamports_per_signature,