From d38228d8b1596bcd61802cfd861cbe7a0385fa72 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Tue, 20 Aug 2024 18:19:43 -0500 Subject: [PATCH 1/4] add another bench full of builtins --- .../process_compute_budget_instructions.rs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/runtime-transaction/benches/process_compute_budget_instructions.rs b/runtime-transaction/benches/process_compute_budget_instructions.rs index 76f6b590948875..ac3f9b6ae892eb 100644 --- a/runtime-transaction/benches/process_compute_budget_instructions.rs +++ b/runtime-transaction/benches/process_compute_budget_instructions.rs @@ -158,6 +158,38 @@ fn bench_process_compute_budget_instructions_mixed(c: &mut Criterion) { ); } +fn bench_process_compute_budget_and_transfer_only(c: &mut Criterion) { + let num_instructions = 355; + let pubkey = Pubkey::new_unique(); + c.benchmark_group("bench_process_compute_budget_and_transfer_only") + .throughput(Throughput::Elements(NUM_TRANSACTIONS_PER_ITER as u64)) + .bench_function( + format!("{num_instructions} transfer instructions and compute budget ixs"), + |bencher| { + let payer_keypair = Keypair::new(); + let mut ixs: Vec<_> = (4..num_instructions) + .map(|i| system_instruction::transfer(&payer_keypair.pubkey(), &pubkey, i)) + .collect(); + ixs.extend(vec![ + ComputeBudgetInstruction::request_heap_frame(40 * 1024), + ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), + ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), + ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), + ]); + let tx = build_sanitized_transaction(&payer_keypair, &ixs); + + bencher.iter(|| { + (0..NUM_TRANSACTIONS_PER_ITER).for_each(|_| { + assert!(process_compute_budget_instructions(black_box( + SVMMessage::program_instructions_iter(&tx) + )) + .is_ok()) + }) + }); + }, + ); +} + criterion_group!( benches, bench_process_compute_budget_instructions_empty, @@ -165,5 +197,6 @@ criterion_group!( bench_process_compute_budget_instructions_compute_budgets, bench_process_compute_budget_instructions_builtins, bench_process_compute_budget_instructions_mixed, + bench_process_compute_budget_and_transfer_only, ); criterion_main!(benches); From 28e6492eb5fa5dc125ce4a89849cab52d492ffeb Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Wed, 21 Aug 2024 18:21:09 -0500 Subject: [PATCH 2/4] collect builtin instructions cost and count; rename compute_budget_instruction_details to instruction_details as it contains more than just compute-budget ix info; --- Cargo.lock | 1 + runtime-transaction/Cargo.toml | 1 + ...tion_details.rs => instruction_details.rs} | 160 ++++++++++++------ .../src/instructions_processor.rs | 5 +- runtime-transaction/src/lib.rs | 2 +- 5 files changed, 116 insertions(+), 53 deletions(-) rename runtime-transaction/src/{compute_budget_instruction_details.rs => instruction_details.rs} (72%) diff --git a/Cargo.lock b/Cargo.lock index 90d37815cebba9..0cce52ac94d161 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7412,6 +7412,7 @@ dependencies = [ "rustc_version 0.4.0", "solana-builtins-default-costs", "solana-compute-budget", + "solana-compute-budget-program", "solana-program", "solana-sdk", "solana-svm-transaction", diff --git a/runtime-transaction/Cargo.toml b/runtime-transaction/Cargo.toml index 69d172ec112c7c..9b31a6e0a3db4d 100644 --- a/runtime-transaction/Cargo.toml +++ b/runtime-transaction/Cargo.toml @@ -26,6 +26,7 @@ name = "solana_runtime_transaction" bincode = { workspace = true } criterion = { workspace = true } rand = { workspace = true } +solana-compute-budget-program = { workspace = true } solana-program = { workspace = true } [package.metadata.docs.rs] diff --git a/runtime-transaction/src/compute_budget_instruction_details.rs b/runtime-transaction/src/instruction_details.rs similarity index 72% rename from runtime-transaction/src/compute_budget_instruction_details.rs rename to runtime-transaction/src/instruction_details.rs index ab148ef14ae152..889badc9fb68ba 100644 --- a/runtime-transaction/src/compute_budget_instruction_details.rs +++ b/runtime-transaction/src/instruction_details.rs @@ -1,5 +1,6 @@ use { crate::compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, + solana_builtins_default_costs::{BUILTIN_INSTRUCTION_COSTS, MAYBE_BUILTIN_KEY}, solana_compute_budget::compute_budget_limits::*, solana_sdk::{ borsh1::try_from_slice_unchecked, @@ -15,29 +16,49 @@ use { #[cfg_attr(test, derive(Eq, PartialEq))] #[derive(Default, Debug)] -pub(crate) struct ComputeBudgetInstructionDetails { +pub(crate) struct InstructionDetails { // compute-budget instruction details: // the first field in tuple is instruction index, second field is the unsanitized value set by user requested_compute_unit_limit: Option<(u8, u32)>, requested_compute_unit_price: Option<(u8, u64)>, requested_heap_size: Option<(u8, u32)>, requested_loaded_accounts_data_size_limit: Option<(u8, u32)>, - num_non_compute_budget_instructions: u32, + // builtin instructions counts and costs + num_compute_budget_instructions: u32, + num_builtin_instructions: u32, + num_non_builtin_instructions: u32, + builtin_instructions_cost: u32, } -impl ComputeBudgetInstructionDetails { +impl InstructionDetails { pub fn try_from<'a>( instructions: impl Iterator)>, ) -> Result { let mut filter = ComputeBudgetProgramIdFilter::new(); - let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default(); + let mut compute_budget_instruction_details = InstructionDetails::default(); for (i, (program_id, instruction)) in instructions.enumerate() { - if filter.is_compute_budget_program(instruction.program_id_index as usize, program_id) { + if filter.is_compute_budget_program(instruction.program_id_index as usize, program_id) + { compute_budget_instruction_details.process_instruction(i as u8, &instruction)?; + saturating_add_assign!( + compute_budget_instruction_details.num_compute_budget_instructions, + 1 + ); + } + + if let Some(cost) = BUILTIN_INSTRUCTION_COSTS.get(program_id) { + saturating_add_assign!( + compute_budget_instruction_details.builtin_instructions_cost, + *cost as u32 + ); + saturating_add_assign!( + compute_budget_instruction_details.num_builtin_instructions, + 1 + ); } else { saturating_add_assign!( - compute_budget_instruction_details.num_non_compute_budget_instructions, + compute_budget_instruction_details.num_non_builtin_instructions, 1 ); } @@ -68,7 +89,7 @@ impl ComputeBudgetInstructionDetails { .requested_compute_unit_limit .map_or_else( || { - self.num_non_compute_budget_instructions + self.num_non_compute_budget_instructions() .saturating_mul(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT) }, |(_index, requested_compute_unit_limit)| requested_compute_unit_limit, @@ -136,9 +157,17 @@ impl ComputeBudgetInstructionDetails { Ok(()) } + #[inline] fn sanitize_requested_heap_size(bytes: u32) -> bool { (MIN_HEAP_FRAME_BYTES..=MAX_HEAP_FRAME_BYTES).contains(&bytes) && bytes % 1024 == 0 } + + #[inline] + fn num_non_compute_budget_instructions(&self) -> u32 { + self.num_builtin_instructions + .saturating_add(self.num_non_builtin_instructions) + .saturating_sub(self.num_compute_budget_instructions) + } } #[cfg(test)] @@ -171,13 +200,16 @@ mod test { ComputeBudgetInstruction::request_heap_frame(40 * 1024), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = InstructionDetails { requested_heap_size: Some((1, 40 * 1024)), - num_non_compute_budget_instructions: 2, - ..ComputeBudgetInstructionDetails::default() + num_compute_budget_instructions: 1, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, + builtin_instructions_cost: solana_compute_budget_program::DEFAULT_COMPUTE_UNITS as u32, + ..InstructionDetails::default() }; assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Ok(expected_details) ); @@ -187,7 +219,7 @@ mod test { ComputeBudgetInstruction::request_heap_frame(41 * 1024), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -199,13 +231,16 @@ mod test { ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = InstructionDetails { requested_compute_unit_limit: Some((1, u32::MAX)), - num_non_compute_budget_instructions: 2, - ..ComputeBudgetInstructionDetails::default() + num_compute_budget_instructions: 1, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, + builtin_instructions_cost: solana_compute_budget_program::DEFAULT_COMPUTE_UNITS as u32, + ..InstructionDetails::default() }; assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Ok(expected_details) ); @@ -215,7 +250,7 @@ mod test { ComputeBudgetInstruction::set_compute_unit_limit(u32::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -227,13 +262,16 @@ mod test { ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = InstructionDetails { requested_compute_unit_price: Some((1, u64::MAX)), - num_non_compute_budget_instructions: 2, - ..ComputeBudgetInstructionDetails::default() + num_compute_budget_instructions: 1, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, + builtin_instructions_cost: solana_compute_budget_program::DEFAULT_COMPUTE_UNITS as u32, + ..InstructionDetails::default() }; assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Ok(expected_details) ); @@ -243,7 +281,7 @@ mod test { ComputeBudgetInstruction::set_compute_unit_price(u64::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -255,13 +293,16 @@ mod test { ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), Instruction::new_with_bincode(Pubkey::new_unique(), &(), vec![]), ]); - let expected_details = ComputeBudgetInstructionDetails { + let expected_details = InstructionDetails { requested_loaded_accounts_data_size_limit: Some((1, u32::MAX)), - num_non_compute_budget_instructions: 2, - ..ComputeBudgetInstructionDetails::default() + num_compute_budget_instructions: 1, + num_builtin_instructions: 1, + num_non_builtin_instructions: 2, + builtin_instructions_cost: solana_compute_budget_program::DEFAULT_COMPUTE_UNITS as u32, + ..InstructionDetails::default() }; assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Ok(expected_details) ); @@ -271,7 +312,7 @@ mod test { ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(u32::MAX), ]); assert_eq!( - ComputeBudgetInstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), + InstructionDetails::try_from(SVMMessage::program_instructions_iter(&tx)), Err(TransactionError::DuplicateInstruction(2)) ); } @@ -279,7 +320,7 @@ mod test { #[test] fn test_sanitize_and_convert_to_compute_budget_limits() { // empty details, default ComputeBudgetLimits with 0 compute_unit_limits - let instruction_details = ComputeBudgetInstructionDetails::default(); + let instruction_details = InstructionDetails::default(); assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), Ok(ComputeBudgetLimits { @@ -288,15 +329,15 @@ mod test { }) ); - let num_non_compute_budget_instructions = 4; - // no compute-budget instructions, all default ComputeBudgetLimits except cu-limit - let instruction_details = ComputeBudgetInstructionDetails { - num_non_compute_budget_instructions, - ..ComputeBudgetInstructionDetails::default() + let instruction_details = InstructionDetails { + num_compute_budget_instructions: 0, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; - let expected_compute_unit_limit = - num_non_compute_budget_instructions * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT; + let expected_compute_unit_limit = instruction_details.num_non_compute_budget_instructions() + * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), Ok(ComputeBudgetLimits { @@ -310,12 +351,15 @@ mod test { InstructionError::InvalidInstructionData, )); // invalid: requested_heap_size can't be zero - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, 0)), requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, 0)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), @@ -323,12 +367,15 @@ mod test { ); // invalid: requested_heap_size can't be less than MIN_HEAP_FRAME_BYTES - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, 0)), requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MIN_HEAP_FRAME_BYTES - 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), @@ -336,12 +383,15 @@ mod test { ); // invalid: requested_heap_size can't be more than MAX_HEAP_FRAME_BYTES - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, 0)), requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MAX_HEAP_FRAME_BYTES + 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), @@ -349,12 +399,15 @@ mod test { ); // invalid: requested_heap_size must be round by 1024 - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, 0)), requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, MIN_HEAP_FRAME_BYTES + 1024 + 1)), requested_loaded_accounts_data_size_limit: Some((4, 1024)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), @@ -362,12 +415,15 @@ mod test { ); // invalid: loaded_account_data_size can't be zero - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, 0)), requested_compute_unit_price: Some((2, 0)), requested_heap_size: Some((3, 40 * 1024)), requested_loaded_accounts_data_size_limit: Some((4, 0)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), @@ -375,12 +431,15 @@ mod test { ); // valid: acceptable MAX - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, u32::MAX)), requested_compute_unit_price: Some((2, u64::MAX)), requested_heap_size: Some((3, MAX_HEAP_FRAME_BYTES)), requested_loaded_accounts_data_size_limit: Some((4, u32::MAX)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), @@ -394,12 +453,15 @@ mod test { // valid let val: u32 = 1024 * 40; - let instruction_details = ComputeBudgetInstructionDetails { + let instruction_details = InstructionDetails { requested_compute_unit_limit: Some((1, val)), requested_compute_unit_price: Some((2, val as u64)), requested_heap_size: Some((3, val)), requested_loaded_accounts_data_size_limit: Some((4, val)), - num_non_compute_budget_instructions, + num_compute_budget_instructions: 4, + num_builtin_instructions: 1, + num_non_builtin_instructions: 3, + ..InstructionDetails::default() }; assert_eq!( instruction_details.sanitize_and_convert_to_compute_budget_limits(), diff --git a/runtime-transaction/src/instructions_processor.rs b/runtime-transaction/src/instructions_processor.rs index 1edba220096276..20ee62b58706e1 100644 --- a/runtime-transaction/src/instructions_processor.rs +++ b/runtime-transaction/src/instructions_processor.rs @@ -1,5 +1,5 @@ use { - crate::compute_budget_instruction_details::*, + crate::instruction_details::*, solana_compute_budget::compute_budget_limits::*, solana_sdk::{pubkey::Pubkey, transaction::TransactionError}, solana_svm_transaction::instruction::SVMInstruction, @@ -13,8 +13,7 @@ use { pub fn process_compute_budget_instructions<'a>( instructions: impl Iterator)>, ) -> Result { - ComputeBudgetInstructionDetails::try_from(instructions)? - .sanitize_and_convert_to_compute_budget_limits() + InstructionDetails::try_from(instructions)?.sanitize_and_convert_to_compute_budget_limits() } #[cfg(test)] diff --git a/runtime-transaction/src/lib.rs b/runtime-transaction/src/lib.rs index 28d54b4eb3b6b8..a909afd31ad4de 100644 --- a/runtime-transaction/src/lib.rs +++ b/runtime-transaction/src/lib.rs @@ -1,8 +1,8 @@ #![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] -mod compute_budget_instruction_details; mod compute_budget_program_id_filter; +mod instruction_details; pub mod instructions_processor; pub mod runtime_transaction; pub mod transaction_meta; From 24ccfdbe0ec79056afee388f1a0a18e8be790e23 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Wed, 21 Aug 2024 18:55:13 -0500 Subject: [PATCH 3/4] rename Filter to builtin_auxiliary_data_store, caches looked-up builtin cost --- Cargo.lock | 1 + runtime-transaction/Cargo.toml | 1 + .../src/builtin_auxiliary_data_store.rs | 104 ++++++++++++++++++ .../src/compute_budget_program_id_filter.rs | 37 ------- .../src/instruction_details.rs | 26 ++--- runtime-transaction/src/lib.rs | 2 +- 6 files changed, 120 insertions(+), 51 deletions(-) create mode 100644 runtime-transaction/src/builtin_auxiliary_data_store.rs delete mode 100644 runtime-transaction/src/compute_budget_program_id_filter.rs diff --git a/Cargo.lock b/Cargo.lock index 0cce52ac94d161..f7a11ce9966f57 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7413,6 +7413,7 @@ dependencies = [ "solana-builtins-default-costs", "solana-compute-budget", "solana-compute-budget-program", + "solana-loader-v4-program", "solana-program", "solana-sdk", "solana-svm-transaction", diff --git a/runtime-transaction/Cargo.toml b/runtime-transaction/Cargo.toml index 9b31a6e0a3db4d..124918ac7a1bdc 100644 --- a/runtime-transaction/Cargo.toml +++ b/runtime-transaction/Cargo.toml @@ -27,6 +27,7 @@ bincode = { workspace = true } criterion = { workspace = true } rand = { workspace = true } solana-compute-budget-program = { workspace = true } +solana-loader-v4-program = { workspace = true } solana-program = { workspace = true } [package.metadata.docs.rs] diff --git a/runtime-transaction/src/builtin_auxiliary_data_store.rs b/runtime-transaction/src/builtin_auxiliary_data_store.rs new file mode 100644 index 00000000000000..140a2ac4d61223 --- /dev/null +++ b/runtime-transaction/src/builtin_auxiliary_data_store.rs @@ -0,0 +1,104 @@ +// static account keys has max +use { + agave_transaction_view::static_account_keys_meta::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE, + solana_builtins_default_costs::{BUILTIN_INSTRUCTION_COSTS, MAYBE_BUILTIN_KEY}, + solana_sdk::pubkey::Pubkey, +}; + +pub(crate) struct BuiltinAuxiliaryDataStore { + // Array of auxiliary data for all possible static and sanitized program_id_index, + // Each possible value of data indicates: + // None - un-checked + // Some - checked, not builtin + // Some> - checked, is builtin and (is-compute-budget, default-cost) + auxiliary_data: [Option>; FILTER_SIZE as usize], +} + +impl BuiltinAuxiliaryDataStore { + pub(crate) fn new() -> Self { + BuiltinAuxiliaryDataStore { + auxiliary_data: [None; FILTER_SIZE as usize], + } + } + + #[inline] + pub(crate) fn get_auxiliary_data( + &mut self, + index: usize, + program_id: &Pubkey, + ) -> Option<(bool, u32)> { + *self + .auxiliary_data + .get_mut(index) + .expect("program id index is sanitized") + .get_or_insert_with(|| { + if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] { + return None; + } + + BUILTIN_INSTRUCTION_COSTS.get(program_id).map(|cost| { + ( + solana_sdk::compute_budget::check_id(program_id), + *cost as u32, + ) + }) + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + + const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111"; + + #[test] + fn test_get_auxiliary_data() { + let mut test_store = BuiltinAuxiliaryDataStore::new(); + let mut index = 9; + + // initial state is Unchecked (eg, None) + assert!(test_store.auxiliary_data[index].is_none()); + + // non builtin returns None + assert!(test_store + .get_auxiliary_data(index, &DUMMY_PROGRAM_ID.parse().unwrap()) + .is_none()); + // but its state is now checked (eg, Some(...)) + assert_eq!(test_store.auxiliary_data[index], Some(None)); + // lookup same `index` will return cached auxiliary data, will *not* lookup `program_id` + // again + assert!(test_store + .get_auxiliary_data(index, &solana_sdk::loader_v4::id()) + .is_none()); + + // builtin return default cost + index += 1; + assert_eq!( + test_store.get_auxiliary_data(index, &solana_sdk::loader_v4::id()), + Some(( + false, + solana_loader_v4_program::DEFAULT_COMPUTE_UNITS as u32 + )) + ); + + // compute-budget return default cost, and true flag + index += 1; + assert_eq!( + test_store.get_auxiliary_data(index, &solana_sdk::compute_budget::id()), + Some(( + true, + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS as u32 + )) + ); + } + + #[test] + #[should_panic(expected = "program id index is sanitized")] + fn test_get_auxiliary_data_out_of_bound_index() { + let mut test_store = BuiltinAuxiliaryDataStore::new(); + assert!(test_store + .get_auxiliary_data(FILTER_SIZE as usize + 1, &DUMMY_PROGRAM_ID.parse().unwrap()) + .is_none()); + } +} diff --git a/runtime-transaction/src/compute_budget_program_id_filter.rs b/runtime-transaction/src/compute_budget_program_id_filter.rs deleted file mode 100644 index b89b67113de105..00000000000000 --- a/runtime-transaction/src/compute_budget_program_id_filter.rs +++ /dev/null @@ -1,37 +0,0 @@ -// static account keys has max -use { - agave_transaction_view::static_account_keys_meta::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE, - solana_builtins_default_costs::MAYBE_BUILTIN_KEY, solana_sdk::pubkey::Pubkey, -}; - -pub(crate) struct ComputeBudgetProgramIdFilter { - // array of slots for all possible static and sanitized program_id_index, - // each slot indicates if a program_id_index has not been checked (eg, None), - // or already checked with result (eg, Some(result)) that can be reused. - flags: [Option; FILTER_SIZE as usize], -} - -impl ComputeBudgetProgramIdFilter { - pub(crate) fn new() -> Self { - ComputeBudgetProgramIdFilter { - flags: [None; FILTER_SIZE as usize], - } - } - - #[inline] - pub(crate) fn is_compute_budget_program(&mut self, index: usize, program_id: &Pubkey) -> bool { - *self - .flags - .get_mut(index) - .expect("program id index is sanitized") - .get_or_insert_with(|| Self::check_program_id(program_id)) - } - - #[inline] - fn check_program_id(program_id: &Pubkey) -> bool { - if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] { - return false; - } - solana_sdk::compute_budget::check_id(program_id) - } -} diff --git a/runtime-transaction/src/instruction_details.rs b/runtime-transaction/src/instruction_details.rs index 889badc9fb68ba..9ffbec098923bb 100644 --- a/runtime-transaction/src/instruction_details.rs +++ b/runtime-transaction/src/instruction_details.rs @@ -1,6 +1,5 @@ use { - crate::compute_budget_program_id_filter::ComputeBudgetProgramIdFilter, - solana_builtins_default_costs::{BUILTIN_INSTRUCTION_COSTS, MAYBE_BUILTIN_KEY}, + crate::builtin_auxiliary_data_store::BuiltinAuxiliaryDataStore, solana_compute_budget::compute_budget_limits::*, solana_sdk::{ borsh1::try_from_slice_unchecked, @@ -34,23 +33,24 @@ impl InstructionDetails { pub fn try_from<'a>( instructions: impl Iterator)>, ) -> Result { - let mut filter = ComputeBudgetProgramIdFilter::new(); + let mut filter = BuiltinAuxiliaryDataStore::new(); let mut compute_budget_instruction_details = InstructionDetails::default(); for (i, (program_id, instruction)) in instructions.enumerate() { - if filter.is_compute_budget_program(instruction.program_id_index as usize, program_id) + if let Some((is_compute_budget, cost)) = + filter.get_auxiliary_data(instruction.program_id_index as usize, program_id) { - compute_budget_instruction_details.process_instruction(i as u8, &instruction)?; - saturating_add_assign!( - compute_budget_instruction_details.num_compute_budget_instructions, - 1 - ); - } - - if let Some(cost) = BUILTIN_INSTRUCTION_COSTS.get(program_id) { + if is_compute_budget { + compute_budget_instruction_details + .process_instruction(i as u8, &instruction)?; + saturating_add_assign!( + compute_budget_instruction_details.num_compute_budget_instructions, + 1 + ); + } saturating_add_assign!( compute_budget_instruction_details.builtin_instructions_cost, - *cost as u32 + cost ); saturating_add_assign!( compute_budget_instruction_details.num_builtin_instructions, diff --git a/runtime-transaction/src/lib.rs b/runtime-transaction/src/lib.rs index a909afd31ad4de..d982bf976dfae5 100644 --- a/runtime-transaction/src/lib.rs +++ b/runtime-transaction/src/lib.rs @@ -1,7 +1,7 @@ #![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] -mod compute_budget_program_id_filter; +mod builtin_auxiliary_data_store; mod instruction_details; pub mod instructions_processor; pub mod runtime_transaction; From d76291d6af7170e630bb6a3357de9b11231f217b Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Thu, 22 Aug 2024 13:44:08 -0500 Subject: [PATCH 4/4] add enum BuiltinCheckStatus --- .../process_compute_budget_instructions.rs | 2 +- .../src/builtin_auxiliary_data_store.rs | 66 ++++++++++++------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/runtime-transaction/benches/process_compute_budget_instructions.rs b/runtime-transaction/benches/process_compute_budget_instructions.rs index ac3f9b6ae892eb..d49d37485192f1 100644 --- a/runtime-transaction/benches/process_compute_budget_instructions.rs +++ b/runtime-transaction/benches/process_compute_budget_instructions.rs @@ -167,7 +167,7 @@ fn bench_process_compute_budget_and_transfer_only(c: &mut Criterion) { format!("{num_instructions} transfer instructions and compute budget ixs"), |bencher| { let payer_keypair = Keypair::new(); - let mut ixs: Vec<_> = (4..num_instructions) + let mut ixs: Vec<_> = (0..num_instructions - 4) .map(|i| system_instruction::transfer(&payer_keypair.pubkey(), &pubkey, i)) .collect(); ixs.extend(vec![ diff --git a/runtime-transaction/src/builtin_auxiliary_data_store.rs b/runtime-transaction/src/builtin_auxiliary_data_store.rs index 140a2ac4d61223..093083a5fee24f 100644 --- a/runtime-transaction/src/builtin_auxiliary_data_store.rs +++ b/runtime-transaction/src/builtin_auxiliary_data_store.rs @@ -5,19 +5,25 @@ use { solana_sdk::pubkey::Pubkey, }; +#[derive(Default, PartialEq)] +enum BuiltinCheckStatus { + #[default] + Unchecked, + NotBuiltin, + Builtin { + is_compute_budget: bool, + default_cost: u32, + }, +} + pub(crate) struct BuiltinAuxiliaryDataStore { - // Array of auxiliary data for all possible static and sanitized program_id_index, - // Each possible value of data indicates: - // None - un-checked - // Some - checked, not builtin - // Some> - checked, is builtin and (is-compute-budget, default-cost) - auxiliary_data: [Option>; FILTER_SIZE as usize], + auxiliary_data: [BuiltinCheckStatus; FILTER_SIZE as usize], } impl BuiltinAuxiliaryDataStore { pub(crate) fn new() -> Self { BuiltinAuxiliaryDataStore { - auxiliary_data: [None; FILTER_SIZE as usize], + auxiliary_data: core::array::from_fn(|_| BuiltinCheckStatus::default()), } } @@ -27,21 +33,37 @@ impl BuiltinAuxiliaryDataStore { index: usize, program_id: &Pubkey, ) -> Option<(bool, u32)> { - *self + let stat = self .auxiliary_data .get_mut(index) - .expect("program id index is sanitized") - .get_or_insert_with(|| { - if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] { - return None; - } + .expect("program id index is sanitized"); + if stat == &BuiltinCheckStatus::Unchecked { + *stat = Self::check_status(program_id) + } - BUILTIN_INSTRUCTION_COSTS.get(program_id).map(|cost| { - ( - solana_sdk::compute_budget::check_id(program_id), - *cost as u32, - ) - }) + match stat { + BuiltinCheckStatus::NotBuiltin => None, + BuiltinCheckStatus::Builtin { + is_compute_budget, + default_cost, + } => Some((*is_compute_budget, *default_cost)), + _ => unreachable!("already checked"), + } + } + + #[inline] + fn check_status(program_id: &Pubkey) -> BuiltinCheckStatus { + if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] { + return BuiltinCheckStatus::NotBuiltin; + } + + BUILTIN_INSTRUCTION_COSTS + .get(program_id) + .map_or(BuiltinCheckStatus::NotBuiltin, |cost| { + BuiltinCheckStatus::Builtin { + is_compute_budget: solana_sdk::compute_budget::check_id(program_id), + default_cost: *cost as u32, + } }) } } @@ -57,15 +79,15 @@ mod test { let mut test_store = BuiltinAuxiliaryDataStore::new(); let mut index = 9; - // initial state is Unchecked (eg, None) - assert!(test_store.auxiliary_data[index].is_none()); + // initial state is Unchecked + assert!(test_store.auxiliary_data[index] == BuiltinCheckStatus::Unchecked); // non builtin returns None assert!(test_store .get_auxiliary_data(index, &DUMMY_PROGRAM_ID.parse().unwrap()) .is_none()); // but its state is now checked (eg, Some(...)) - assert_eq!(test_store.auxiliary_data[index], Some(None)); + assert!(test_store.auxiliary_data[index] == BuiltinCheckStatus::NotBuiltin); // lookup same `index` will return cached auxiliary data, will *not* lookup `program_id` // again assert!(test_store