Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions runtime/src/bank/check_transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -146,22 +149,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.
Comment thread
t-nelson marked this conversation as resolved.
if lamports_per_signature == 0 {
compute_budget_and_limits.fee_details = FeeDetails::default();
}

CheckedTransactionDetails::new(nonce, compute_budget_and_limits)
}

Expand All @@ -173,7 +168,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) {
Expand Down
8 changes: 4 additions & 4 deletions svm/src/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,28 +70,28 @@ 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")]
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(),
}),
},
}
}
}

impl CheckedTransactionDetails {
pub fn new(
nonce: Option<NonceInfo>,
compute_budget_and_limits: Result<SVMTransactionExecutionAndFeeBudgetLimits>,
compute_budget_and_limits: SVMTransactionExecutionAndFeeBudgetLimits,
) -> Self {
Self {
nonce,
Expand Down
59 changes: 14 additions & 45 deletions svm/src/transaction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,10 +625,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
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -1982,7 +1978,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,
Expand Down Expand Up @@ -2061,7 +2057,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,
Expand Down Expand Up @@ -2114,7 +2110,7 @@ mod tests {
&message,
CheckedTransactionDetails::new(
None,
Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()),
SVMTransactionExecutionAndFeeBudgetLimits::default(),
),
&Hash::default(),
&Rent::default(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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::<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) {
Expand Down Expand Up @@ -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::<TestForkGraph>::validate_transaction_nonce_and_fee_payer(
Expand Down Expand Up @@ -2404,7 +2373,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,
Expand Down Expand Up @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions svm/tests/concurrent_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 14 additions & 12 deletions svm/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
});

Expand Down Expand Up @@ -554,7 +556,7 @@ impl TransactionBatchItem {
Self {
check_result: Ok(CheckedTransactionDetails::new(
Some(nonce_info),
Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()),
SVMTransactionExecutionAndFeeBudgetLimits::default(),
)),
..Self::default()
}
Expand All @@ -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(),
}
Expand Down
Loading