From 0d9b7161e1fb6d5e6081dad6fefe172704d4defd Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Mon, 5 Jun 2023 17:31:48 +0000 Subject: [PATCH 1/6] 1. update cost_model and cost_tracker logic for feature gate native_programs_consume_cu status; 2. TransactionCost to combine builtin and bpf execution cost into programs_execution_cost; 3. enhance tests to cover feature gate native_programs_consume_cu status --- core/src/banking_stage/qos_service.rs | 233 ++++++++++-------- runtime/src/cost_model.rs | 330 ++++++++++++++++++++------ runtime/src/cost_tracker.rs | 88 +++++-- runtime/src/transaction_cost.rs | 10 + 4 files changed, 467 insertions(+), 194 deletions(-) diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index abe6d691666b6e..87a650bfa954a0 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -209,9 +209,8 @@ impl QosService { // checked for update if let Ok(tx_cost) = tx_cost { match transaction_committed_details { - CommitTransactionDetails::Committed { compute_units } => { - cost_tracker.update_execution_cost(tx_cost, *compute_units) - } + CommitTransactionDetails::Committed { compute_units } => cost_tracker + .update_execution_cost(tx_cost, *compute_units, &bank.feature_set), CommitTransactionDetails::NotCommitted => cost_tracker.remove(tx_cost), } } @@ -627,6 +626,7 @@ mod tests { itertools::Itertools, solana_runtime::genesis_utils::{create_genesis_config, GenesisConfigInfo}, solana_sdk::{ + feature_set::native_programs_consume_cu, hash::Hash, signature::{Keypair, Signer}, system_transaction, @@ -733,7 +733,6 @@ mod tests { fn test_update_or_remove_transaction_costs_commited() { solana_logger::setup(); let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10); - let bank = Arc::new(Bank::new_for_tests(&genesis_config)); // make some transfer transactions // calculate their costs, apply to cost_tracker @@ -747,46 +746,64 @@ mod tests { .collect(); let execute_units_adjustment = 10u64; - // assert all tx_costs should be applied to cost_tracker if all execution_results are all committed - { - let qos_service = QosService::new(1); - let txs_costs = qos_service.compute_transaction_costs( - &FeatureSet::all_enabled(), - txs.iter(), - std::iter::repeat(Ok(())), - ); - let total_txs_cost: u64 = txs_costs - .iter() - .map(|cost| cost.as_ref().unwrap().sum()) - .sum(); - let (qos_cost_results, _num_included) = - qos_service.select_transactions_per_cost(txs.iter(), txs_costs.into_iter(), &bank); - assert_eq!( - total_txs_cost, - bank.read_cost_tracker().unwrap().block_cost() - ); - // all transactions are committed with actual units more than estimated - let commited_status: Vec = qos_cost_results - .iter() - .map(|tx_cost| CommitTransactionDetails::Committed { - compute_units: tx_cost.as_ref().unwrap().bpf_execution_cost - + execute_units_adjustment, - }) - .collect(); - let final_txs_cost = total_txs_cost + execute_units_adjustment * transaction_count; - QosService::update_or_remove_transaction_costs( - qos_cost_results.iter(), - Some(&commited_status), - &bank, - ); - assert_eq!( - final_txs_cost, - bank.read_cost_tracker().unwrap().block_cost() - ); - assert_eq!( - transaction_count, - bank.read_cost_tracker().unwrap().transaction_count() - ); + for native_programs_consume_cu_feature_activated in [true, false] { + let mut bank = Bank::new_for_tests(&genesis_config); + if native_programs_consume_cu_feature_activated { + bank.activate_feature(&native_programs_consume_cu::id()); + } else { + bank.deactivate_feature(&native_programs_consume_cu::id()); + } + let bank = Arc::new(bank); + // assert all tx_costs should be applied to cost_tracker if all execution_results are all committed + { + let qos_service = QosService::new(1); + let txs_costs = qos_service.compute_transaction_costs( + &FeatureSet::all_enabled(), + txs.iter(), + std::iter::repeat(Ok(())), + ); + let total_txs_cost: u64 = txs_costs + .iter() + .map(|cost| cost.as_ref().unwrap().sum()) + .sum(); + let (qos_cost_results, _num_included) = qos_service.select_transactions_per_cost( + txs.iter(), + txs_costs.into_iter(), + &bank, + ); + assert_eq!( + total_txs_cost, + bank.read_cost_tracker().unwrap().block_cost() + ); + // all transactions are committed with actual units more than estimated + let commited_status: Vec = qos_cost_results + .iter() + .map(|tx_cost| { + let actual_consumed_cu = if native_programs_consume_cu_feature_activated { + tx_cost.as_ref().unwrap().programs_execution_cost() + } else { + tx_cost.as_ref().unwrap().bpf_execution_cost + } + execute_units_adjustment; + CommitTransactionDetails::Committed { + compute_units: actual_consumed_cu, + } + }) + .collect(); + let final_txs_cost = total_txs_cost + execute_units_adjustment * transaction_count; + QosService::update_or_remove_transaction_costs( + qos_cost_results.iter(), + Some(&commited_status), + &bank, + ); + assert_eq!( + final_txs_cost, + bank.read_cost_tracker().unwrap().block_cost() + ); + assert_eq!( + transaction_count, + bank.read_cost_tracker().unwrap().transaction_count() + ); + } } } @@ -835,7 +852,6 @@ mod tests { fn test_update_or_remove_transaction_costs_mixed_execution() { solana_logger::setup(); let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10); - let bank = Arc::new(Bank::new_for_tests(&genesis_config)); // make some transfer transactions // calculate their costs, apply to cost_tracker @@ -849,63 +865,80 @@ mod tests { .collect(); let execute_units_adjustment = 10u64; - // assert only commited tx_costs are applied cost_tracker - { - let qos_service = QosService::new(1); - let txs_costs = qos_service.compute_transaction_costs( - &FeatureSet::all_enabled(), - txs.iter(), - std::iter::repeat(Ok(())), - ); - let total_txs_cost: u64 = txs_costs - .iter() - .map(|cost| cost.as_ref().unwrap().sum()) - .sum(); - let (qos_cost_results, _num_included) = - qos_service.select_transactions_per_cost(txs.iter(), txs_costs.into_iter(), &bank); - assert_eq!( - total_txs_cost, - bank.read_cost_tracker().unwrap().block_cost() - ); - // Half of transactions are not committed, the rest with cost adjustment - let commited_status: Vec = qos_cost_results - .iter() - .enumerate() - .map(|(n, tx_cost)| { - if n % 2 == 0 { - CommitTransactionDetails::NotCommitted - } else { - CommitTransactionDetails::Committed { - compute_units: tx_cost.as_ref().unwrap().bpf_execution_cost - + execute_units_adjustment, + for native_programs_consume_cu_feature_activated in [true, false] { + let mut bank = Bank::new_for_tests(&genesis_config); + if native_programs_consume_cu_feature_activated { + bank.activate_feature(&native_programs_consume_cu::id()); + } else { + bank.deactivate_feature(&native_programs_consume_cu::id()); + } + let bank = Arc::new(bank); + // assert only commited tx_costs are applied cost_tracker + { + let qos_service = QosService::new(1); + let txs_costs = qos_service.compute_transaction_costs( + &FeatureSet::all_enabled(), + txs.iter(), + std::iter::repeat(Ok(())), + ); + let total_txs_cost: u64 = txs_costs + .iter() + .map(|cost| cost.as_ref().unwrap().sum()) + .sum(); + let (qos_cost_results, _num_included) = qos_service.select_transactions_per_cost( + txs.iter(), + txs_costs.into_iter(), + &bank, + ); + assert_eq!( + total_txs_cost, + bank.read_cost_tracker().unwrap().block_cost() + ); + // Half of transactions are not committed, the rest with cost adjustment + let commited_status: Vec = qos_cost_results + .iter() + .enumerate() + .map(|(n, tx_cost)| { + if n % 2 == 0 { + CommitTransactionDetails::NotCommitted + } else { + let actual_consumed_cu = if native_programs_consume_cu_feature_activated + { + tx_cost.as_ref().unwrap().programs_execution_cost() + } else { + tx_cost.as_ref().unwrap().bpf_execution_cost + } + execute_units_adjustment; + CommitTransactionDetails::Committed { + compute_units: actual_consumed_cu, + } } - } - }) - .collect(); - QosService::update_or_remove_transaction_costs( - qos_cost_results.iter(), - Some(&commited_status), - &bank, - ); + }) + .collect(); + QosService::update_or_remove_transaction_costs( + qos_cost_results.iter(), + Some(&commited_status), + &bank, + ); - // assert the final block cost - let mut expected_final_txs_count = 0u64; - let mut expected_final_block_cost = 0u64; - qos_cost_results.iter().enumerate().for_each(|(n, cost)| { - if n % 2 != 0 { - expected_final_txs_count += 1; - expected_final_block_cost += - cost.as_ref().unwrap().sum() + execute_units_adjustment; - } - }); - assert_eq!( - expected_final_block_cost, - bank.read_cost_tracker().unwrap().block_cost() - ); - assert_eq!( - expected_final_txs_count, - bank.read_cost_tracker().unwrap().transaction_count() - ); + // assert the final block cost + let mut expected_final_txs_count = 0u64; + let mut expected_final_block_cost = 0u64; + qos_cost_results.iter().enumerate().for_each(|(n, cost)| { + if n % 2 != 0 { + expected_final_txs_count += 1; + expected_final_block_cost += + cost.as_ref().unwrap().sum() + execute_units_adjustment; + } + }); + assert_eq!( + expected_final_block_cost, + bank.read_cost_tracker().unwrap().block_cost() + ); + assert_eq!( + expected_final_txs_count, + bank.read_cost_tracker().unwrap().transaction_count() + ); + } } } diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index 1ab8b80dc016ce..0d7f7fbba37a0a 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -14,7 +14,7 @@ use { solana_sdk::{ feature_set::{ add_set_tx_loaded_accounts_data_size_instruction, - include_loaded_accounts_data_size_in_fee_calculation, + include_loaded_accounts_data_size_in_fee_calculation, native_programs_consume_cu, remove_deprecated_request_unit_ix, FeatureSet, }, instruction::CompiledInstruction, @@ -126,9 +126,16 @@ impl CostModel { // by `bank`, therefore it should be considered as no execution cost by cost model. match result { Ok(_) => { - // if tx contained user-space instructions and a more accurate estimate available correct it - if bpf_costs > 0 { - bpf_costs = compute_budget.compute_unit_limit + if feature_set.is_active(&native_programs_consume_cu::id()) { + // if feature gate is activated, the requested compute_unit_limit covers both + // builtin and bpf programs + builtin_costs = compute_budget.compute_unit_limit.min(builtin_costs); + bpf_costs = compute_budget.compute_unit_limit - builtin_costs; + } else { + // if tx contained user-space instructions and a more accurate estimate available correct it + if bpf_costs > 0 { + bpf_costs = compute_budget.compute_unit_limit + } } if feature_set .is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()) @@ -225,6 +232,8 @@ mod tests { std::sync::Arc, }; + const DEFAULT_PAGE_COST: u64 = 8; + fn test_setup() -> (Keypair, Hash) { solana_logger::setup(); let GenesisConfigInfo { @@ -298,19 +307,28 @@ mod tests { ); // expected cost for one system transfer instructions - let expected_execution_cost = BUILT_IN_INSTRUCTION_COSTS + let expected_builtin_cost = *BUILT_IN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap(); - let mut tx_cost = TransactionCost::default(); - CostModel::get_transaction_cost( - &mut tx_cost, - &simple_transaction, - &FeatureSet::all_enabled(), - ); - assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost); - assert_eq!(0, tx_cost.bpf_execution_cost); - assert_eq!(3, tx_cost.data_bytes_cost); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let (expected_execution_cost, expected_bpf_cost) = + if feature_set.is_active(&native_programs_consume_cu::id()) { + ( + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64, + DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 - expected_builtin_cost, + ) + } else { + (expected_builtin_cost, 0) // no bpf instructions, therefore no bpf cost + }; + + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost(&mut tx_cost, &simple_transaction, &feature_set); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(expected_builtin_cost, tx_cost.builtins_execution_cost); + assert_eq!(expected_bpf_cost, tx_cost.bpf_execution_cost); + assert_eq!(3, tx_cost.data_bytes_cost); + } } #[test] @@ -329,28 +347,104 @@ mod tests { instructions, ); let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); - debug!("token_transaction {:?}", token_transaction); + let expected_cost = + solana_program_runtime::compute_budget::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64; + + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost(&mut tx_cost, &token_transaction, &feature_set); + assert_eq!(expected_cost, tx_cost.programs_execution_cost()); + assert_eq!(0, tx_cost.builtins_execution_cost); + assert_eq!(expected_cost, tx_cost.bpf_execution_cost); + assert_eq!(0, tx_cost.data_bytes_cost); + } + } - let mut tx_cost = TransactionCost::default(); - CostModel::get_transaction_cost( - &mut tx_cost, - &token_transaction, - &FeatureSet::all_enabled(), + #[test] + fn test_cost_model_transaction_cost_with_compute_budget() { + let (mint_keypair, start_hash) = test_setup(); + let request_cu_limit = 12_345_u64; + let compute_budget_program_cost = *BUILT_IN_INSTRUCTION_COSTS + .get(&compute_budget::id()) + .unwrap(); + + let instructions = vec![ + CompiledInstruction::new(3, &(), vec![1, 2, 0]), + CompiledInstruction::new_from_raw_parts( + 4, + ComputeBudgetInstruction::SetComputeUnitLimit(request_cu_limit as u32) + .pack() + .unwrap(), + vec![], + ), + ]; + let tx = Transaction::new_with_compiled_instructions( + &[&mint_keypair], + &[ + solana_sdk::pubkey::new_rand(), + solana_sdk::pubkey::new_rand(), + ], + start_hash, + vec![inline_spl_token::id(), compute_budget::id()], + instructions, ); - assert_eq!(0, tx_cost.builtins_execution_cost); - assert_eq!(200_000, tx_cost.bpf_execution_cost); - assert_eq!(0, tx_cost.data_bytes_cost); + let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); + + // When `native_programs_consume_cu` feature gate is not activated, requested cu_limit + // applies to only bpf programs. + { + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost( + &mut tx_cost, + &token_transaction, + &FeatureSet::default(), + ); + assert_eq!( + compute_budget_program_cost + request_cu_limit, + tx_cost.programs_execution_cost() + ); + assert_eq!(compute_budget_program_cost, tx_cost.builtins_execution_cost); + assert_eq!(request_cu_limit, tx_cost.bpf_execution_cost); + assert_eq!(1, tx_cost.data_bytes_cost); + } + + // When `native_programs_consume_cu` feature gate is activated, requested cu_limit + // applies to both builtin and bpf program. + { + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost( + &mut tx_cost, + &token_transaction, + &FeatureSet::all_enabled(), + ); + assert_eq!(request_cu_limit, tx_cost.programs_execution_cost()); + assert_eq!(compute_budget_program_cost, tx_cost.builtins_execution_cost); + assert_eq!( + request_cu_limit - compute_budget_program_cost, + tx_cost.bpf_execution_cost + ); + assert_eq!(1, tx_cost.data_bytes_cost); + } } #[test] - fn test_cost_model_compute_budget_transaction() { + fn test_cost_model_transaction_cost_with_failed_compute_budget() { let (mint_keypair, start_hash) = test_setup(); + let request_cu_limit = 12_345_u64; let instructions = vec![ CompiledInstruction::new(3, &(), vec![1, 2, 0]), CompiledInstruction::new_from_raw_parts( 4, - ComputeBudgetInstruction::SetComputeUnitLimit(12_345) + ComputeBudgetInstruction::SetComputeUnitLimit(request_cu_limit as u32) + .pack() + .unwrap(), + vec![], + ), + // to trigger `duplicate_instruction_error` error + CompiledInstruction::new_from_raw_parts( + 4, + ComputeBudgetInstruction::SetComputeUnitLimit(1_000) .pack() .unwrap(), vec![], @@ -368,20 +462,93 @@ mod tests { ); let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); - let mut tx_cost = TransactionCost::default(); - CostModel::get_transaction_cost( - &mut tx_cost, - &token_transaction, - &FeatureSet::all_enabled(), - ); - assert_eq!( - *BUILT_IN_INSTRUCTION_COSTS - .get(&compute_budget::id()) - .unwrap(), - tx_cost.builtins_execution_cost + // compute_budget program will fail to process instructions due to error + // `duplicate_instruction_error`. The transaction will not be sent to runtime + // for execution, hence its execution_cost and loaded_account_data_size_cost + // will all be zero. + let expected_execution_cost = 0; + let expected_loaded_account_data_size_cost = 0; + + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost(&mut tx_cost, &token_transaction, &feature_set); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!( + expected_loaded_account_data_size_cost, + tx_cost.loaded_accounts_data_size_cost + ); + } + } + + #[test] + fn test_cost_model_transaction_cost_without_compute_budget() { + let (mint_keypair, start_hash) = test_setup(); + let builtin_cost = *BUILT_IN_INSTRUCTION_COSTS + .get(&compute_budget::id()) + .unwrap(); + let default_bpf_cost = + solana_program_runtime::compute_budget::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64; + + let instructions = vec![ + CompiledInstruction::new(3, &(), vec![1, 2, 0]), + CompiledInstruction::new_from_raw_parts( + 4, + ComputeBudgetInstruction::RequestHeapFrame(32 * 1024u32) + .pack() + .unwrap(), + vec![], + ), + ]; + let tx = Transaction::new_with_compiled_instructions( + &[&mint_keypair], + &[ + solana_sdk::pubkey::new_rand(), + solana_sdk::pubkey::new_rand(), + ], + start_hash, + vec![inline_spl_token::id(), compute_budget::id()], + instructions, ); - assert_eq!(12_345, tx_cost.bpf_execution_cost); - assert_eq!(1, tx_cost.data_bytes_cost); + let token_transaction = SanitizedTransaction::from_transaction_for_tests(tx); + + // With all features disabled, and without compute_unit_limit instruction, + { + let expected_execution_cost = builtin_cost + default_bpf_cost; + let expected_loaded_accounts_data_size_cost = 0u64; + + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost( + &mut tx_cost, + &token_transaction, + &FeatureSet::default(), + ); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!( + expected_loaded_accounts_data_size_cost, + tx_cost.loaded_accounts_data_size_cost + ); + } + + // With all features activated, and without compute_unit_limit instruction, + { + let expected_execution_cost = default_bpf_cost; + let expected_loaded_accounts_data_size_cost = + solana_program_runtime::compute_budget::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES as u64 + / ACCOUNT_DATA_COST_PAGE_SIZE + * DEFAULT_PAGE_COST; + + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost( + &mut tx_cost, + &token_transaction, + &FeatureSet::all_enabled(), + ); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!( + expected_loaded_accounts_data_size_cost, + tx_cost.loaded_accounts_data_size_cost + ); + } } #[test] @@ -424,8 +591,10 @@ mod tests { &token_transaction, &FeatureSet::all_enabled(), ); + assert_eq!(0, tx_cost.programs_execution_cost()); assert_eq!(0, tx_cost.builtins_execution_cost); assert_eq!(0, tx_cost.bpf_execution_cost); + assert_eq!(0, tx_cost.loaded_accounts_data_size_cost); } #[test] @@ -442,19 +611,28 @@ mod tests { message, start_hash, )); - debug!("many transfer transaction {:?}", tx); // expected cost for two system transfer instructions - let program_cost = BUILT_IN_INSTRUCTION_COSTS + let expected_builtin_cost = *BUILT_IN_INSTRUCTION_COSTS .get(&system_program::id()) - .unwrap(); - let expected_cost = program_cost * 2; - - let mut tx_cost = TransactionCost::default(); - CostModel::get_transaction_cost(&mut tx_cost, &tx, &FeatureSet::all_enabled()); - assert_eq!(expected_cost, tx_cost.builtins_execution_cost); - assert_eq!(0, tx_cost.bpf_execution_cost); - assert_eq!(6, tx_cost.data_bytes_cost); + .unwrap() + * 2; + + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let (expected_execution_cost, expected_bpf_cost) = + if feature_set.is_active(&native_programs_consume_cu::id()) { + let tx_execution_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2; // two instructions + (tx_execution_cost, tx_execution_cost - expected_builtin_cost) + } else { + (expected_builtin_cost, 0) // no bpf instructions, therefore no bpf cost + }; + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost(&mut tx_cost, &tx, &feature_set); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(expected_builtin_cost, tx_cost.builtins_execution_cost); + assert_eq!(expected_bpf_cost, tx_cost.bpf_execution_cost); + assert_eq!(6, tx_cost.data_bytes_cost); + } } #[test] @@ -479,14 +657,16 @@ mod tests { instructions, ), ); - debug!("many random transaction {:?}", tx); let expected_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64 * 2; - let mut tx_cost = TransactionCost::default(); - CostModel::get_transaction_cost(&mut tx_cost, &tx, &FeatureSet::all_enabled()); - assert_eq!(0, tx_cost.builtins_execution_cost); - assert_eq!(expected_cost, tx_cost.bpf_execution_cost); - assert_eq!(0, tx_cost.data_bytes_cost); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let mut tx_cost = TransactionCost::default(); + CostModel::get_transaction_cost(&mut tx_cost, &tx, &feature_set); + assert_eq!(expected_cost, tx_cost.programs_execution_cost()); + assert_eq!(0, tx_cost.builtins_execution_cost); + assert_eq!(expected_cost, tx_cost.bpf_execution_cost); + assert_eq!(0, tx_cost.data_bytes_cost); + } } #[test] @@ -512,16 +692,18 @@ mod tests { ), ); - let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); - assert_eq!(2 + 2, tx_cost.writable_accounts.len()); - assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]); - assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]); - assert_eq!(key1, tx_cost.writable_accounts[2]); - assert_eq!(key2, tx_cost.writable_accounts[3]); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let tx_cost = CostModel::calculate_cost(&tx, &feature_set); + assert_eq!(2 + 2, tx_cost.writable_accounts.len()); + assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]); + assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]); + assert_eq!(key1, tx_cost.writable_accounts[2]); + assert_eq!(key2, tx_cost.writable_accounts[3]); + } } #[test] - fn test_cost_model_calculate_cost_all_default() { + fn test_cost_model_calculate_cost_all_enabled() { let (mint_keypair, start_hash) = test_setup(); let tx = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( &mint_keypair, @@ -531,12 +713,12 @@ mod tests { )); let expected_account_cost = WRITE_LOCK_UNITS * 2; - let expected_execution_cost = BUILT_IN_INSTRUCTION_COSTS + let expected_execution_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64; + let expected_builtin_cost = *BUILT_IN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap(); // feature `include_loaded_accounts_data_size_in_fee_calculation` enabled, using // default loaded_accounts_data_size_limit - const DEFAULT_PAGE_COST: u64 = 8; let expected_loaded_accounts_data_size_cost = solana_program_runtime::compute_budget::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES as u64 / ACCOUNT_DATA_COST_PAGE_SIZE @@ -544,8 +726,9 @@ mod tests { let tx_cost = CostModel::calculate_cost(&tx, &FeatureSet::all_enabled()); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); - assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost); assert_eq!(2, tx_cost.writable_accounts.len()); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(expected_builtin_cost, tx_cost.builtins_execution_cost); assert_eq!( expected_loaded_accounts_data_size_cost, tx_cost.loaded_accounts_data_size_cost @@ -565,7 +748,7 @@ mod tests { let feature_set = FeatureSet::default(); assert!(!feature_set.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id())); let expected_account_cost = WRITE_LOCK_UNITS * 2; - let expected_execution_cost = BUILT_IN_INSTRUCTION_COSTS + let expected_execution_cost = *BUILT_IN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap(); // feature `include_loaded_accounts_data_size_in_fee_calculation` not enabled @@ -573,7 +756,8 @@ mod tests { let tx_cost = CostModel::calculate_cost(&tx, &feature_set); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); - assert_eq!(*expected_execution_cost, tx_cost.builtins_execution_cost); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(expected_execution_cost, tx_cost.builtins_execution_cost); assert_eq!(2, tx_cost.writable_accounts.len()); assert_eq!( expected_loaded_accounts_data_size_cost, @@ -600,19 +784,24 @@ mod tests { let feature_set = FeatureSet::all_enabled(); assert!(feature_set.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id())); let expected_account_cost = WRITE_LOCK_UNITS * 2; - let expected_execution_cost = BUILT_IN_INSTRUCTION_COSTS + // `native_programs_consume_cu` is activated, so requested cu_limit is for both builtin and + // bpf programs execution cost + let expected_execution_cost = DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64; + let expected_builtin_cost = *BUILT_IN_INSTRUCTION_COSTS .get(&system_program::id()) .unwrap() - + BUILT_IN_INSTRUCTION_COSTS + + *BUILT_IN_INSTRUCTION_COSTS .get(&compute_budget::id()) .unwrap(); // feature `include_loaded_accounts_data_size_in_fee_calculation` is enabled, accounts data // size limit is set. - let expected_loaded_accounts_data_size_cost = (data_limit as u64) / (32 * 1024) * 8; + let expected_loaded_accounts_data_size_cost = + (data_limit as u64) / ACCOUNT_DATA_COST_PAGE_SIZE * DEFAULT_PAGE_COST; let tx_cost = CostModel::calculate_cost(&tx, &feature_set); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); - assert_eq!(expected_execution_cost, tx_cost.builtins_execution_cost); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); + assert_eq!(expected_builtin_cost, tx_cost.builtins_execution_cost); assert_eq!(2, tx_cost.writable_accounts.len()); assert_eq!( expected_loaded_accounts_data_size_cost, @@ -645,6 +834,7 @@ mod tests { let tx_cost = CostModel::calculate_cost(&tx, &feature_set); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); + assert_eq!(expected_execution_cost, tx_cost.programs_execution_cost()); assert_eq!(expected_execution_cost, tx_cost.builtins_execution_cost); assert_eq!(2, tx_cost.writable_accounts.len()); assert_eq!( diff --git a/runtime/src/cost_tracker.rs b/runtime/src/cost_tracker.rs index a07afa3367c47f..72ee8cd2ae49d4 100644 --- a/runtime/src/cost_tracker.rs +++ b/runtime/src/cost_tracker.rs @@ -6,7 +6,11 @@ use { crate::{block_cost_limits::*, transaction_cost::TransactionCost}, solana_sdk::{ - clock::Slot, pubkey::Pubkey, saturating_add_assign, transaction::TransactionError, + clock::Slot, + feature_set::{native_programs_consume_cu, FeatureSet}, + pubkey::Pubkey, + saturating_add_assign, + transaction::TransactionError, }, std::{cmp::Ordering, collections::HashMap}, }; @@ -117,8 +121,17 @@ impl CostTracker { &mut self, estimated_tx_cost: &TransactionCost, actual_execution_units: u64, + feature_set: &FeatureSet, ) { - let estimated_execution_units = estimated_tx_cost.bpf_execution_cost; + // when feature gate `native_programs_consume_cu` is activated, `runtime` will consume + // compute units for both bpf and builtin program. Therefore the `actual_execution_units` + // will include builtin_execution_costs as well. + let estimated_execution_units = if feature_set.is_active(&native_programs_consume_cu::id()) + { + estimated_tx_cost.programs_execution_cost() + } else { + estimated_tx_cost.bpf_execution_cost + }; match actual_execution_units.cmp(&estimated_execution_units) { Ordering::Equal => (), Ordering::Greater => { @@ -818,11 +831,14 @@ mod tests { let acct1 = Pubkey::new_unique(); let acct2 = Pubkey::new_unique(); let acct3 = Pubkey::new_unique(); - let cost = 100; + let builtin_cost: u64 = 10; + let bpf_cost: u64 = 90; + let execution_cost = builtin_cost + bpf_cost; let tx_cost = TransactionCost { writable_accounts: vec![acct1, acct2, acct3], - bpf_execution_cost: cost, + builtins_execution_cost: builtin_cost, + bpf_execution_cost: bpf_cost, ..TransactionCost::default() }; @@ -831,35 +847,59 @@ mod tests { // Assert OK to add tx_cost assert!(cost_tracker.try_add(&tx_cost).is_ok()); let (_costliest_account, costliest_account_cost) = cost_tracker.find_costliest_account(); - assert_eq!(cost, cost_tracker.block_cost); - assert_eq!(cost, costliest_account_cost); + assert_eq!(execution_cost, cost_tracker.block_cost); + assert_eq!(execution_cost, costliest_account_cost); assert_eq!(1, cost_tracker.transaction_count); // assert no-change if actual units is same as estimated units - let mut expected_cost = cost; - cost_tracker.update_execution_cost(&tx_cost, cost); - let (_costliest_account, costliest_account_cost) = cost_tracker.find_costliest_account(); - assert_eq!(expected_cost, cost_tracker.block_cost); - assert_eq!(expected_cost, costliest_account_cost); - assert_eq!(1, cost_tracker.transaction_count); + let mut expected_cost = execution_cost; + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let actual_consumed_cu = if feature_set.is_active(&native_programs_consume_cu::id()) { + builtin_cost + bpf_cost + } else { + bpf_cost + }; + cost_tracker.update_execution_cost(&tx_cost, actual_consumed_cu, &feature_set); + let (_costliest_account, costliest_account_cost) = + cost_tracker.find_costliest_account(); + assert_eq!(expected_cost, cost_tracker.block_cost); + assert_eq!(expected_cost, costliest_account_cost); + assert_eq!(1, cost_tracker.transaction_count); + } // assert cost are adjusted down let reduced_units = 3; - expected_cost -= reduced_units; - cost_tracker.update_execution_cost(&tx_cost, cost - reduced_units); - let (_costliest_account, costliest_account_cost) = cost_tracker.find_costliest_account(); - assert_eq!(expected_cost, cost_tracker.block_cost); - assert_eq!(expected_cost, costliest_account_cost); - assert_eq!(1, cost_tracker.transaction_count); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let actual_consumed_cu = if feature_set.is_active(&native_programs_consume_cu::id()) { + builtin_cost + bpf_cost + } else { + bpf_cost + } - reduced_units; + expected_cost -= reduced_units; + cost_tracker.update_execution_cost(&tx_cost, actual_consumed_cu, &feature_set); + let (_costliest_account, costliest_account_cost) = + cost_tracker.find_costliest_account(); + assert_eq!(expected_cost, cost_tracker.block_cost); + assert_eq!(expected_cost, costliest_account_cost); + assert_eq!(1, cost_tracker.transaction_count); + } // assert cost are adjusted up let increased_units = 1; - expected_cost += increased_units; - cost_tracker.update_execution_cost(&tx_cost, cost + increased_units); - let (_costliest_account, costliest_account_cost) = cost_tracker.find_costliest_account(); - assert_eq!(expected_cost, cost_tracker.block_cost); - assert_eq!(expected_cost, costliest_account_cost); - assert_eq!(1, cost_tracker.transaction_count); + for feature_set in [FeatureSet::default(), FeatureSet::all_enabled()] { + let actual_consumed_cu = if feature_set.is_active(&native_programs_consume_cu::id()) { + builtin_cost + bpf_cost + } else { + bpf_cost + } + increased_units; + expected_cost += increased_units; + cost_tracker.update_execution_cost(&tx_cost, actual_consumed_cu, &feature_set); + let (_costliest_account, costliest_account_cost) = + cost_tracker.find_costliest_account(); + assert_eq!(expected_cost, cost_tracker.block_cost); + assert_eq!(expected_cost, costliest_account_cost); + assert_eq!(1, cost_tracker.transaction_count); + } } #[test] diff --git a/runtime/src/transaction_cost.rs b/runtime/src/transaction_cost.rs index e44014b3c934d7..0b14f0eff3cc54 100644 --- a/runtime/src/transaction_cost.rs +++ b/runtime/src/transaction_cost.rs @@ -9,6 +9,11 @@ pub struct TransactionCost { pub signature_cost: u64, pub write_lock_cost: u64, pub data_bytes_cost: u64, + // Note: Once feature gate `native_programs_consume_cu` is activated in mnb, + // it won't be necessary to track builtin and bpf programs' execution + // cost separately, the requested compute_unit_limit will apply to + // both programs. `builtins_execution_cost` and `bpf_execution_cost` + // can be combined into `programs_execution_cost`. pub builtins_execution_cost: u64, pub bpf_execution_cost: u64, pub loaded_accounts_data_size_cost: u64, @@ -74,4 +79,9 @@ impl TransactionCost { .saturating_add(self.bpf_execution_cost) .saturating_add(self.loaded_accounts_data_size_cost) } + + pub fn programs_execution_cost(&self) -> u64 { + self.builtins_execution_cost + .saturating_add(self.bpf_execution_cost) + } } From cfe2a6ad50b7f07525c2538411dbbd6d56bd1585 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Mon, 12 Jun 2023 23:21:26 +0000 Subject: [PATCH 2/6] update a ledger test by adding set_compute_unit_limit ix --- Cargo.lock | 2 ++ ledger/Cargo.toml | 2 ++ ledger/src/blockstore_processor.rs | 19 +++++++++++++++---- runtime/src/cost_model.rs | 4 +++- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8577d393d38af4..4523fe6b5a024a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5978,6 +5978,7 @@ dependencies = [ "sha2 0.10.7", "solana-account-decoder", "solana-bpf-loader-program", + "solana-compute-budget-program", "solana-entry", "solana-frozen-abi", "solana-frozen-abi-macro", @@ -5992,6 +5993,7 @@ dependencies = [ "solana-stake-program", "solana-storage-bigtable", "solana-storage-proto", + "solana-system-program", "solana-transaction-status", "solana-vote-program", "spl-token", diff --git a/ledger/Cargo.toml b/ledger/Cargo.toml index 0bc385ceac3653..f7d3ebe31cd2e4 100644 --- a/ledger/Cargo.toml +++ b/ledger/Cargo.toml @@ -38,6 +38,7 @@ serde_bytes = { workspace = true } sha2 = { workspace = true } solana-account-decoder = { workspace = true } solana-bpf-loader-program = { workspace = true } +solana-compute-budget-program = { workspace = true } solana-entry = { workspace = true } solana-frozen-abi = { workspace = true } solana-frozen-abi-macro = { workspace = true } @@ -51,6 +52,7 @@ solana-sdk = { workspace = true } solana-stake-program = { workspace = true } solana-storage-bigtable = { workspace = true } solana-storage-proto = { workspace = true } +solana-system-program = { workspace = true } solana-transaction-status = { workspace = true } solana-vote-program = { workspace = true } spl-token = { workspace = true, features = ["no-entrypoint"] } diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 99e1da377986d2..a744d9fc5b1848 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3256,15 +3256,26 @@ pub mod tests { let present_account = AccountSharedData::new(1, 10, &Pubkey::default()); bank.store_account(&present_account_key.pubkey(), &present_account); + let compute_unit_limit = solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS + as u32 + + solana_compute_budget_program::DEFAULT_COMPUTE_UNITS as u32; let entries: Vec<_> = (0..NUM_TRANSFERS) .step_by(NUM_TRANSFERS_PER_ENTRY) .map(|i| { let mut transactions = (0..NUM_TRANSFERS_PER_ENTRY) .map(|j| { - system_transaction::transfer( - &keypairs[i + j], - &keypairs[i + j + NUM_TRANSFERS].pubkey(), - 1, + // transactions should request compute_unit_limit + Transaction::new_signed_with_payer( + &[ + solana_sdk::system_instruction::transfer( + &keypairs[i + j].pubkey(), + &keypairs[i + j + NUM_TRANSFERS].pubkey(), + 1, + ), + solana_sdk::compute_budget::ComputeBudgetInstruction::set_compute_unit_limit(compute_unit_limit), + ], + Some(&keypairs[i + j].pubkey()), + &[&keypairs[i + j]], bank.last_blockhash(), ) }) diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index 0d7f7fbba37a0a..344f5efcaaa322 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -130,7 +130,9 @@ impl CostModel { // if feature gate is activated, the requested compute_unit_limit covers both // builtin and bpf programs builtin_costs = compute_budget.compute_unit_limit.min(builtin_costs); - bpf_costs = compute_budget.compute_unit_limit - builtin_costs; + bpf_costs = compute_budget + .compute_unit_limit + .saturating_sub(builtin_costs); } else { // if tx contained user-space instructions and a more accurate estimate available correct it if bpf_costs > 0 { From 33d37e5c47f46d0fdf1c6546d83532d15a600992 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Mon, 12 Jun 2023 23:30:42 +0000 Subject: [PATCH 3/6] uncommit cargo.lock changes --- Cargo.lock | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4523fe6b5a024a..8577d393d38af4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5978,7 +5978,6 @@ dependencies = [ "sha2 0.10.7", "solana-account-decoder", "solana-bpf-loader-program", - "solana-compute-budget-program", "solana-entry", "solana-frozen-abi", "solana-frozen-abi-macro", @@ -5993,7 +5992,6 @@ dependencies = [ "solana-stake-program", "solana-storage-bigtable", "solana-storage-proto", - "solana-system-program", "solana-transaction-status", "solana-vote-program", "spl-token", From c6b459423de2572686446079131aa5f4bb1372ec Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Tue, 13 Jun 2023 00:22:05 +0000 Subject: [PATCH 4/6] recommit cargo.log change --- Cargo.lock | 2 ++ programs/sbf/Cargo.lock | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 8577d393d38af4..4523fe6b5a024a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5978,6 +5978,7 @@ dependencies = [ "sha2 0.10.7", "solana-account-decoder", "solana-bpf-loader-program", + "solana-compute-budget-program", "solana-entry", "solana-frozen-abi", "solana-frozen-abi-macro", @@ -5992,6 +5993,7 @@ dependencies = [ "solana-stake-program", "solana-storage-bigtable", "solana-storage-proto", + "solana-system-program", "solana-transaction-status", "solana-vote-program", "spl-token", diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 7c95954fc8123f..41ea466b10a123 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -4987,6 +4987,7 @@ dependencies = [ "sha2 0.10.7", "solana-account-decoder", "solana-bpf-loader-program", + "solana-compute-budget-program", "solana-entry", "solana-frozen-abi", "solana-frozen-abi-macro", @@ -5000,6 +5001,7 @@ dependencies = [ "solana-stake-program", "solana-storage-bigtable", "solana-storage-proto", + "solana-system-program", "solana-transaction-status", "solana-vote-program", "spl-token", From ea8bd062fdbb65f08a315c6d53facb7282bce7ca Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Mon, 12 Jun 2023 23:30:42 +0000 Subject: [PATCH 5/6] uncommit cargo.lock changes --- Cargo.lock | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4523fe6b5a024a..8577d393d38af4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5978,7 +5978,6 @@ dependencies = [ "sha2 0.10.7", "solana-account-decoder", "solana-bpf-loader-program", - "solana-compute-budget-program", "solana-entry", "solana-frozen-abi", "solana-frozen-abi-macro", @@ -5993,7 +5992,6 @@ dependencies = [ "solana-stake-program", "solana-storage-bigtable", "solana-storage-proto", - "solana-system-program", "solana-transaction-status", "solana-vote-program", "spl-token", From 31c201888254aecf89c1d64d6a3f2d98ec5d8508 Mon Sep 17 00:00:00 2001 From: Tao Zhu Date: Tue, 13 Jun 2023 00:22:05 +0000 Subject: [PATCH 6/6] recommit cargo.log change --- Cargo.lock | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 8577d393d38af4..4523fe6b5a024a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5978,6 +5978,7 @@ dependencies = [ "sha2 0.10.7", "solana-account-decoder", "solana-bpf-loader-program", + "solana-compute-budget-program", "solana-entry", "solana-frozen-abi", "solana-frozen-abi-macro", @@ -5992,6 +5993,7 @@ dependencies = [ "solana-stake-program", "solana-storage-bigtable", "solana-storage-proto", + "solana-system-program", "solana-transaction-status", "solana-vote-program", "spl-token",