diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index d516146dbb2..bb33077519d 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -127,7 +127,10 @@ impl Bank { raise_cpi_limit, ) } - }); + }) + .inspect_err(|_err| { + error_counters.invalid_compute_budget += 1; + })?; self.check_transaction_age( tx.borrow(), max_age, @@ -146,22 +149,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) } @@ -173,7 +168,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 94913bde651..6bf49c56451 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -70,7 +70,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")] @@ -78,12 +78,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(), - }), + }, } } } @@ -91,7 +91,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 ad55a501aaa..f61ab5f62cd 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -625,10 +625,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 @@ -1134,7 +1130,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, }; @@ -1982,7 +1978,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, @@ -2061,7 +2057,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, @@ -2114,7 +2110,7 @@ mod tests { &message, CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), + SVMTransactionExecutionAndFeeBudgetLimits::default(), ), &Hash::default(), &Rent::default(), @@ -2147,13 +2143,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(), @@ -2190,13 +2186,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, @@ -2231,13 +2227,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(), @@ -2248,33 +2244,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) { @@ -2335,7 +2304,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( @@ -2404,7 +2373,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, @@ -2447,9 +2416,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 d5f6c5ee892..c3d9cc8ac27 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -513,16 +513,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) }); @@ -554,7 +556,7 @@ impl TransactionBatchItem { Self { check_result: Ok(CheckedTransactionDetails::new( Some(nonce_info), - Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), + SVMTransactionExecutionAndFeeBudgetLimits::default(), )), ..Self::default() } @@ -567,7 +569,7 @@ impl Default for TransactionBatchItem { transaction: Transaction::default(), check_result: Ok(CheckedTransactionDetails::new( None, - Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), + SVMTransactionExecutionAndFeeBudgetLimits::default(), )), asserts: TransactionBatchItemAsserts::default(), }