runtime: resolve compute budget in `check_transaction()` (#9155)

* runtime: resolve compute budget in `check_transaction()`

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<NonceInfo>,
         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<SVMTransactionExecutionAndFeeBudgetLimits, TransactionError>,
+        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) {

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<NonceInfo>,
-    pub(crate) compute_budget_and_limits: Result<SVMTransactionExecutionAndFeeBudgetLimits>,
+    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<NonceInfo>,
-        compute_budget_and_limits: Result<SVMTransactionExecutionAndFeeBudgetLimits>,
+        compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits,
     ) -> Self {
         Self {
             nonce,

svm/src/transaction_processor.rs

@@ -688,10 +688,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             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::<TestForkGraph>::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::<TestForkGraph>::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::<TestForkGraph>::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::<TestForkGraph>::validate_transaction_nonce_and_fee_payer(
@@ -2467,7 +2436,7 @@ mod tests {
             let result = TransactionBatchProcessor::<TestForkGraph>::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(),

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();

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(),
         }