From 114ba0b039ce626a8333102a98806e3105cbe1d1 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 5 Feb 2025 14:52:43 -0800 Subject: [PATCH] Remove SVM's dep on compute-budget-instruction --- programs/sbf/Cargo.lock | 1 - runtime/src/bank/check_transactions.rs | 5 ++ svm/Cargo.toml | 2 +- svm/examples/Cargo.lock | 1 - .../json-rpc/server/src/rpc_process.rs | 10 ++- svm/examples/paytube/src/processor.rs | 13 +++- svm/src/account_loader.rs | 20 ++++- svm/src/transaction_processor.rs | 78 +++++++++++++------ svm/tests/concurrent_tests.rs | 8 +- svm/tests/integration_test.rs | 8 +- 10 files changed, 112 insertions(+), 34 deletions(-) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index c63a46bedcd094..ab9aad8b49c4e6 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -8192,7 +8192,6 @@ dependencies = [ "solana-bpf-loader-program", "solana-clock", "solana-compute-budget", - "solana-compute-budget-instruction", "solana-feature-set", "solana-fee-structure", "solana-hash", diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index 3c5adc2b3ba116..09333a10d45adc 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -1,6 +1,7 @@ use { super::{Bank, BankStatusCache}, solana_accounts_db::blockhash_queue::BlockhashQueue, + solana_compute_budget_instruction::instructions_processor::process_compute_budget_instructions, solana_perf::perf_libs, solana_runtime_transaction::transaction_with_meta::TransactionWithMeta, solana_sdk::{ @@ -111,11 +112,14 @@ impl Bank { next_lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, ) -> TransactionCheckResult { + let compute_budget_limits = + process_compute_budget_instructions(tx.program_instructions_iter(), &self.feature_set); let recent_blockhash = tx.recent_blockhash(); if let Some(hash_info) = hash_queue.get_hash_info_if_valid(recent_blockhash, max_age) { Ok(CheckedTransactionDetails::new( None, hash_info.lamports_per_signature(), + compute_budget_limits, )) } else if let Some((nonce, previous_lamports_per_signature)) = self .check_load_and_advance_message_nonce_account( @@ -127,6 +131,7 @@ impl Bank { Ok(CheckedTransactionDetails::new( Some(nonce), previous_lamports_per_signature, + compute_budget_limits, )) } else { error_counters.blockhash_not_found += 1; diff --git a/svm/Cargo.toml b/svm/Cargo.toml index 24ddfec6365264..005aa018927e39 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -21,7 +21,6 @@ solana-account = { workspace = true } solana-bpf-loader-program = { workspace = true } solana-clock = { workspace = true } solana-compute-budget = { workspace = true } -solana-compute-budget-instruction = { workspace = true } solana-feature-set = { workspace = true } solana-fee-structure = { workspace = true } solana-frozen-abi = { workspace = true, optional = true, features = [ @@ -71,6 +70,7 @@ rand0-7 = { workspace = true } shuttle = { workspace = true } solana-clock = { workspace = true } solana-compute-budget = { workspace = true, features = ["dev-context-only-utils"] } +solana-compute-budget-instruction = { workspace = true } solana-compute-budget-interface = { workspace = true } solana-compute-budget-program = { workspace = true } solana-ed25519-program = { workspace = true } diff --git a/svm/examples/Cargo.lock b/svm/examples/Cargo.lock index cbc2a0ad3b6325..e4657823b673ac 100644 --- a/svm/examples/Cargo.lock +++ b/svm/examples/Cargo.lock @@ -7520,7 +7520,6 @@ dependencies = [ "solana-bpf-loader-program", "solana-clock", "solana-compute-budget", - "solana-compute-budget-instruction", "solana-feature-set", "solana-fee-structure", "solana-hash", diff --git a/svm/examples/json-rpc/server/src/rpc_process.rs b/svm/examples/json-rpc/server/src/rpc_process.rs index 8d0d51f5263d12..8144cae156759d 100644 --- a/svm/examples/json-rpc/server/src/rpc_process.rs +++ b/svm/examples/json-rpc/server/src/rpc_process.rs @@ -15,7 +15,9 @@ use { parse_token::{get_token_account_mint, is_known_spl_token_id}, UiAccount, UiAccountEncoding, UiDataSliceConfig, MAX_BASE58_BYTES, }, - solana_compute_budget::compute_budget::ComputeBudget, + solana_compute_budget::{ + compute_budget::ComputeBudget, compute_budget_limits::ComputeBudgetLimits, + }, solana_perf::packet::PACKET_DATA_SIZE, solana_program_runtime::loaded_programs::ProgramCacheEntry, solana_rpc_client_api::{ @@ -423,7 +425,11 @@ impl JsonRpcRequestProcessor { _error_counters: &mut TransactionErrorMetrics, ) -> TransactionCheckResult { /* for now just return defaults */ - Ok(CheckedTransactionDetails::new(None, u64::default())) + Ok(CheckedTransactionDetails::new( + None, + u64::default(), + Ok(ComputeBudgetLimits::default()), + )) } fn clock(&self) -> sysvar::clock::Clock { diff --git a/svm/examples/paytube/src/processor.rs b/svm/examples/paytube/src/processor.rs index d917966e370485..da76454075b915 100644 --- a/svm/examples/paytube/src/processor.rs +++ b/svm/examples/paytube/src/processor.rs @@ -2,7 +2,9 @@ use { solana_bpf_loader_program::syscalls::create_program_runtime_environment_v1, - solana_compute_budget::compute_budget::ComputeBudget, + solana_compute_budget::{ + compute_budget::ComputeBudget, compute_budget_limits::ComputeBudgetLimits, + }, solana_program_runtime::loaded_programs::{BlockRelation, ForkGraph, ProgramCacheEntry}, solana_sdk::{clock::Slot, feature_set::FeatureSet, transaction}, solana_svm::{ @@ -92,5 +94,12 @@ pub(crate) fn get_transaction_check_results( len: usize, lamports_per_signature: u64, ) -> Vec> { - vec![transaction::Result::Ok(CheckedTransactionDetails::new(None, lamports_per_signature)); len] + vec![ + transaction::Result::Ok(CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(ComputeBudgetLimits::default()) + )); + len + ] } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 1a4f2bc7cd5f7b..dbc2003a14c584 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -58,17 +58,33 @@ pub(crate) enum TransactionLoadResult { } #[derive(PartialEq, Eq, Debug, Clone)] -#[cfg_attr(feature = "dev-context-only-utils", derive(Default))] pub struct CheckedTransactionDetails { pub(crate) nonce: Option, pub(crate) lamports_per_signature: u64, + pub(crate) compute_budget_limits: Result, +} + +#[cfg(feature = "dev-context-only-utils")] +impl Default for CheckedTransactionDetails { + fn default() -> Self { + Self { + nonce: None, + lamports_per_signature: 0, + compute_budget_limits: Ok(ComputeBudgetLimits::default()), + } + } } impl CheckedTransactionDetails { - pub fn new(nonce: Option, lamports_per_signature: u64) -> Self { + pub fn new( + nonce: Option, + lamports_per_signature: u64, + compute_budget_limits: Result, + ) -> Self { Self { nonce, lamports_per_signature, + compute_budget_limits, } } } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 25227abf196e60..aed4bf84c50643 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -26,7 +26,6 @@ use { }, solana_clock::{Epoch, Slot}, solana_compute_budget::compute_budget::ComputeBudget, - solana_compute_budget_instruction::instructions_processor::process_compute_budget_instructions, solana_feature_set::{ enable_transaction_loading_failure_fees, remove_accounts_executable_flag_checks, FeatureSet, }, @@ -527,6 +526,7 @@ impl TransactionBatchProcessor { if let CheckedTransactionDetails { nonce: Some(ref nonce_info), lamports_per_signature: _, + compute_budget_limits: _, } = checked_details { let next_durable_nonce = DurableNonce::from_blockhash(environment_blockhash); @@ -563,11 +563,13 @@ impl TransactionBatchProcessor { error_counters: &mut TransactionErrorMetrics, callbacks: &CB, ) -> TransactionResult { - let compute_budget_limits = process_compute_budget_instructions( - message.program_instructions_iter(), - &account_loader.feature_set, - ) - .inspect_err(|_err| { + let CheckedTransactionDetails { + nonce, + lamports_per_signature, + compute_budget_limits, + } = checked_details; + + let compute_budget_limits = compute_budget_limits.inspect_err(|_err| { error_counters.invalid_compute_budget += 1; })?; @@ -588,11 +590,6 @@ impl TransactionBatchProcessor { ) .rent_amount; - let CheckedTransactionDetails { - nonce, - lamports_per_signature, - } = checked_details; - let fee_budget_limits = FeeBudgetLimits::from(compute_budget_limits); let fee_details = if lamports_per_signature == 0 { FeeDetails::default() @@ -1208,6 +1205,7 @@ mod tests { solana_account::{create_account_shared_data_for_test, WritableAccount}, solana_clock::Clock, solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, + solana_compute_budget_instruction::instructions_processor::process_compute_budget_instructions, solana_compute_budget_interface::ComputeBudgetInstruction, solana_epoch_schedule::EpochSchedule, solana_feature_set::FeatureSet, @@ -2186,7 +2184,11 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(compute_budget_limits), + ), &Hash::default(), FeeStructure::default().lamports_per_signature, &rent_collector, @@ -2264,7 +2266,11 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(compute_budget_limits), + ), &Hash::default(), FeeStructure::default().lamports_per_signature, &rent_collector, @@ -2313,7 +2319,11 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(ComputeBudgetLimits::default()), + ), &Hash::default(), FeeStructure::default().lamports_per_signature, &RentCollector::default(), @@ -2345,7 +2355,11 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(ComputeBudgetLimits::default()), + ), &Hash::default(), FeeStructure::default().lamports_per_signature, &RentCollector::default(), @@ -2381,7 +2395,11 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(ComputeBudgetLimits::default()), + ), &Hash::default(), FeeStructure::default().lamports_per_signature, &rent_collector, @@ -2415,7 +2433,11 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new( + None, + lamports_per_signature, + Ok(ComputeBudgetLimits::default()), + ), &Hash::default(), FeeStructure::default().lamports_per_signature, &RentCollector::default(), @@ -2437,6 +2459,10 @@ mod tests { ], Some(&Pubkey::new_unique()), )); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ); let mock_bank = MockBankCallback::default(); let mut account_loader = (&mock_bank).into(); @@ -2445,7 +2471,7 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new(None, lamports_per_signature, compute_budget_limits), &Hash::default(), FeeStructure::default().lamports_per_signature, &RentCollector::default(), @@ -2513,8 +2539,11 @@ mod tests { .try_advance_nonce(next_durable_nonce, lamports_per_signature) .unwrap(); - let tx_details = - CheckedTransactionDetails::new(Some(future_nonce.clone()), lamports_per_signature); + let tx_details = CheckedTransactionDetails::new( + Some(future_nonce.clone()), + lamports_per_signature, + Ok(compute_budget_limits), + ); let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, @@ -2578,7 +2607,7 @@ mod tests { let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, lamports_per_signature), + CheckedTransactionDetails::new(None, lamports_per_signature, Ok(compute_budget_limits)), &Hash::default(), FeeStructure::default().lamports_per_signature, &rent_collector, @@ -2618,10 +2647,15 @@ mod tests { Some(&fee_payer_address), &Hash::new_unique(), )); + let compute_budget_limits = process_compute_budget_instructions( + SVMMessage::program_instructions_iter(&message), + &FeatureSet::default(), + ) + .unwrap(); TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, 5000), + CheckedTransactionDetails::new(None, 5000, Ok(compute_budget_limits)), &Hash::default(), FeeStructure::default().lamports_per_signature, &RentCollector::default(), diff --git a/svm/tests/concurrent_tests.rs b/svm/tests/concurrent_tests.rs index 867ed0c7ba3066..649e7009b59d4f 100644 --- a/svm/tests/concurrent_tests.rs +++ b/svm/tests/concurrent_tests.rs @@ -11,6 +11,7 @@ use { sync::{Arc, RwLock}, thread, Runner, }, + solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_program_runtime::loaded_programs::ProgramCacheEntryType, solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, @@ -239,8 +240,11 @@ fn svm_concurrent() { let local_bank = mock_bank.clone(); let th_txs = std::mem::take(&mut transactions[idx]); let check_results = vec![ - Ok(CheckedTransactionDetails::new(None, 20)) - as TransactionCheckResult; + Ok(CheckedTransactionDetails::new( + None, + 20, + Ok(ComputeBudgetLimits::default()) + )) as TransactionCheckResult; TRANSACTIONS_PER_THREAD ]; let processing_config = TransactionProcessingConfig { diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 3a4892bbfe09d5..c8400ac2fd60ef 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -7,6 +7,7 @@ use { register_builtins, MockBankCallback, MockForkGraph, EXECUTION_EPOCH, EXECUTION_SLOT, WALLCLOCK_TIME, }, + solana_compute_budget::compute_budget_limits::ComputeBudgetLimits, solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, clock::Slot, @@ -433,6 +434,7 @@ impl TransactionBatchItem { check_result: Ok(CheckedTransactionDetails::new( Some(nonce_info), LAMPORTS_PER_SIGNATURE, + Ok(ComputeBudgetLimits::default()), )), ..Self::default() } @@ -443,7 +445,11 @@ impl Default for TransactionBatchItem { fn default() -> Self { Self { transaction: Transaction::default(), - check_result: Ok(CheckedTransactionDetails::new(None, LAMPORTS_PER_SIGNATURE)), + check_result: Ok(CheckedTransactionDetails::new( + None, + LAMPORTS_PER_SIGNATURE, + Ok(ComputeBudgetLimits::default()), + )), asserts: TransactionBatchItemAsserts::default(), } }