diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index 18488c0b55f..c48df3ebd2d 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -142,7 +142,10 @@ impl Bank { raise_cpi_limit, ) } - }); + }) + .inspect_err(|_err| { + error_counters.invalid_compute_budget += 1; + })?; self.check_transaction_age( tx.borrow(), max_age, @@ -161,22 +164,14 @@ impl Bank { fn checked_transactions_details_with_test_override( nonce: Option, lamports_per_signature: u64, - compute_budget_and_limits: Result< - SVMTransactionExecutionAndFeeBudgetLimits, - TransactionError, - >, + mut compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, ) -> CheckedTransactionDetails { - let compute_budget_and_limits = if lamports_per_signature == 0 { - // This is done to support legacy tests. The tests should be updated, and check - // for 0 lamports_per_signature should be removed from the code. - compute_budget_and_limits.map(|v| SVMTransactionExecutionAndFeeBudgetLimits { - budget: v.budget, - loaded_accounts_data_size_limit: v.loaded_accounts_data_size_limit, - fee_details: FeeDetails::default(), - }) - } else { - compute_budget_and_limits - }; + // This is done to support legacy tests. The tests should be updated, and check + // for 0 lamports_per_signature should be removed from the code. + if lamports_per_signature == 0 { + compute_budget_and_limits.fee_details = FeeDetails::default(); + } + CheckedTransactionDetails::new(nonce, compute_budget_and_limits) } @@ -188,7 +183,7 @@ impl Bank { hash_queue: &BlockhashQueue, next_lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, - compute_budget: Result, + compute_budget: SVMTransactionExecutionAndFeeBudgetLimits, ) -> TransactionCheckResult { let recent_blockhash = tx.recent_blockhash(); if let Some(hash_info) = hash_queue.get_hash_info_if_valid(recent_blockhash, max_age) { diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 889677bd003..3c7b120ed33 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -69,7 +69,7 @@ pub(crate) enum TransactionLoadResult { #[cfg_attr(feature = "svm-internal", field_qualifiers(nonce(pub)))] pub struct CheckedTransactionDetails { pub(crate) nonce: Option, - pub(crate) compute_budget_and_limits: Result, + pub(crate) compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, } #[cfg(feature = "dev-context-only-utils")] @@ -77,12 +77,12 @@ impl Default for CheckedTransactionDetails { fn default() -> Self { Self { nonce: None, - compute_budget_and_limits: Ok(SVMTransactionExecutionAndFeeBudgetLimits { + compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits { budget: SVMTransactionExecutionBudget::default(), loaded_accounts_data_size_limit: NonZeroU32::new(32) .expect("Failed to set loaded_accounts_bytes"), fee_details: FeeDetails::default(), - }), + }, } } } @@ -90,7 +90,7 @@ impl Default for CheckedTransactionDetails { impl CheckedTransactionDetails { pub fn new( nonce: Option, - compute_budget_and_limits: Result, + compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits, ) -> Self { Self { nonce, diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 6706f3e3fa6..cf9ff12e5d5 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -688,10 +688,6 @@ impl TransactionBatchProcessor { compute_budget_and_limits, } = checked_details; - let compute_budget_and_limits = compute_budget_and_limits.inspect_err(|_err| { - error_counters.invalid_compute_budget += 1; - })?; - let fee_payer_address = message.fee_payer(); // We *must* use load_transaction_account() here because *this* is when the fee-payer @@ -1197,7 +1193,7 @@ mod tests { solana_svm_callback::{AccountState, InvokeContextCallback}, solana_transaction::{sanitized::SanitizedTransaction, Transaction}, solana_transaction_context::TransactionContext, - solana_transaction_error::{TransactionError, TransactionError::DuplicateInstruction}, + solana_transaction_error::TransactionError, std::collections::HashMap, test_case::test_case, }; @@ -2045,7 +2041,7 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, Ok(compute_budget_and_limits)), + CheckedTransactionDetails::new(None, compute_budget_and_limits), &Hash::default(), &rent, &mut error_counters, @@ -2124,7 +2120,7 @@ mod tests { TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, Ok(compute_budget_and_limits)), + CheckedTransactionDetails::new(None, compute_budget_and_limits), &Hash::default(), &rent, &mut error_counters, @@ -2177,7 +2173,7 @@ mod tests { &message, CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), + SVMTransactionExecutionAndFeeBudgetLimits::default(), ), &Hash::default(), &Rent::default(), @@ -2210,13 +2206,13 @@ mod tests { &message, CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::with_fee( + SVMTransactionExecutionAndFeeBudgetLimits::with_fee( MockBankCallback::calculate_fee_details( &message, lamports_per_signature, 0, ), - )), + ), ), &Hash::default(), &Rent::default(), @@ -2253,13 +2249,13 @@ mod tests { &message, CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::with_fee( + SVMTransactionExecutionAndFeeBudgetLimits::with_fee( MockBankCallback::calculate_fee_details( &message, lamports_per_signature, 0, ), - )), + ), ), &Hash::default(), &rent, @@ -2294,13 +2290,13 @@ mod tests { &message, CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::with_fee( + SVMTransactionExecutionAndFeeBudgetLimits::with_fee( MockBankCallback::calculate_fee_details( &message, lamports_per_signature, 0, ), - )), + ), ), &Hash::default(), &Rent::default(), @@ -2311,33 +2307,6 @@ mod tests { assert_eq!(result, Err(TransactionError::InvalidAccountForFee)); } - #[test] - fn test_validate_transaction_fee_payer_invalid_compute_budget() { - let message = new_unchecked_sanitized_message(Message::new( - &[ - ComputeBudgetInstruction::set_compute_unit_limit(2000u32), - ComputeBudgetInstruction::set_compute_unit_limit(42u32), - ], - Some(&Pubkey::new_unique()), - )); - - let mock_bank = MockBankCallback::default(); - let mut account_loader = (&mock_bank).into(); - let mut error_counters = TransactionErrorMetrics::default(); - let result = - TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( - &mut account_loader, - &message, - CheckedTransactionDetails::new(None, Err(DuplicateInstruction(1))), - &Hash::default(), - &Rent::default(), - &mut error_counters, - ); - - assert_eq!(error_counters.invalid_compute_budget.0, 1); - assert_eq!(result, Err(TransactionError::DuplicateInstruction(1u8))); - } - #[test_case(false; "informal_loaded_size")] #[test_case(true; "simd186_loaded_size")] fn test_validate_transaction_fee_payer_is_nonce(formalize_loaded_transaction_data_size: bool) { @@ -2398,7 +2367,7 @@ mod tests { let tx_details = CheckedTransactionDetails::new( Some(future_nonce.clone()), - Ok(compute_budget_and_limits), + compute_budget_and_limits, ); let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( @@ -2467,7 +2436,7 @@ mod tests { let result = TransactionBatchProcessor::::validate_transaction_nonce_and_fee_payer( &mut account_loader, &message, - CheckedTransactionDetails::new(None, Ok(compute_budget_and_limits)), + CheckedTransactionDetails::new(None, compute_budget_and_limits), &Hash::default(), &rent, &mut error_counters, @@ -2510,9 +2479,9 @@ mod tests { &message, CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::with_fee( + SVMTransactionExecutionAndFeeBudgetLimits::with_fee( MockBankCallback::calculate_fee_details(&message, 5000, 0), - )), + ), ), &Hash::default(), &Rent::default(), diff --git a/svm/tests/concurrent_tests.rs b/svm/tests/concurrent_tests.rs index cd6e71b99dc..a3b8b7c0648 100644 --- a/svm/tests/concurrent_tests.rs +++ b/svm/tests/concurrent_tests.rs @@ -241,9 +241,9 @@ fn svm_concurrent() { .map(|tx| { Ok(CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::with_fee( + SVMTransactionExecutionAndFeeBudgetLimits::with_fee( MockBankCallback::calculate_fee_details(tx, 0), - )), + ), )) as TransactionCheckResult }) .collect(); diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 2e65dc52c49..84e4fe2a783 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -526,16 +526,18 @@ impl SvmTestEntry { .saturating_add(message.num_secp256k1_signatures()) .saturating_add(message.num_secp256r1_signatures()); - let compute_budget = compute_budget_limits.map(|v| { - v.get_compute_budget_and_limits( - v.loaded_accounts_bytes, - FeeDetails::new( - signature_count.saturating_mul(LAMPORTS_PER_SIGNATURE), - v.get_prioritization_fee(), - ), - self.feature_set.raise_cpi_nesting_limit_to_8, - ) - }); + let compute_budget = compute_budget_limits + .map(|v| { + v.get_compute_budget_and_limits( + v.loaded_accounts_bytes, + FeeDetails::new( + signature_count.saturating_mul(LAMPORTS_PER_SIGNATURE), + v.get_prioritization_fee(), + ), + self.feature_set.raise_cpi_nesting_limit_to_8, + ) + }) + .unwrap(); CheckedTransactionDetails::new(tx_details.nonce, compute_budget) }); @@ -567,7 +569,7 @@ impl TransactionBatchItem { Self { check_result: Ok(CheckedTransactionDetails::new( Some(nonce_info), - Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), + SVMTransactionExecutionAndFeeBudgetLimits::default(), )), ..Self::default() } @@ -580,7 +582,7 @@ impl Default for TransactionBatchItem { transaction: Transaction::default(), check_result: Ok(CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), + SVMTransactionExecutionAndFeeBudgetLimits::default(), )), asserts: TransactionBatchItemAsserts::default(), }