From be5337a839806e5026290ccc93bab657f240f44d Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Fri, 12 Jan 2024 08:22:39 -0800 Subject: [PATCH] earlier fee payer validation (#34666) --- core/src/banking_stage/consumer.rs | 63 +++++++++++++++++-- .../unprocessed_transaction_storage.rs | 25 ++++++++ runtime/src/accounts/mod.rs | 2 +- 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 69639264d..64b688897 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -19,14 +19,19 @@ use { BankStart, PohRecorderError, RecordTransactionsSummary, RecordTransactionsTimings, TransactionRecorder, }, - solana_program_runtime::timings::ExecuteTimings, + solana_program_runtime::{ + compute_budget_processor::process_compute_budget_instructions, timings::ExecuteTimings, + }, solana_runtime::{ + accounts::validate_fee_payer, bank::{Bank, LoadAndExecuteTransactionsOutput}, transaction_batch::TransactionBatch, }, solana_sdk::{ clock::{Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE}, - feature_set, saturating_add_assign, + feature_set, + message::SanitizedMessage, + saturating_add_assign, timing::timestamp, transaction::{self, AddressLoader, SanitizedTransaction, TransactionError}, }, @@ -394,9 +399,24 @@ impl Consumer { txs: &[SanitizedTransaction], chunk_offset: usize, ) -> ProcessTransactionBatchOutput { - // No filtering before QoS - transactions should have been sanitized immediately prior to this call - let pre_results = std::iter::repeat(Ok(())); - self.process_and_record_transactions_with_pre_results(bank, txs, chunk_offset, pre_results) + let mut error_counters = TransactionErrorMetrics::default(); + let pre_results = vec![Ok(()); txs.len()]; + let check_results = + bank.check_transactions(txs, &pre_results, MAX_PROCESSING_AGE, &mut error_counters); + let check_results = check_results.into_iter().map(|(result, _nonce)| result); + let mut output = self.process_and_record_transactions_with_pre_results( + bank, + txs, + chunk_offset, + check_results, + ); + + // Accumulate error counters from the initial checks into final results + output + .execute_and_commit_transactions_output + .error_counters + .accumulate(&error_counters); + output } pub fn process_and_record_aged_transactions( @@ -684,6 +704,39 @@ impl Consumer { } } + pub fn check_fee_payer_unlocked( + bank: &Bank, + message: &SanitizedMessage, + error_counters: &mut TransactionErrorMetrics, + ) -> Result<(), TransactionError> { + let fee_payer = message.fee_payer(); + let budget_limits = + process_compute_budget_instructions(message.program_instructions_iter())?.into(); + let fee = bank.fee_structure.calculate_fee( + message, + bank.get_lamports_per_signature(), + &budget_limits, + bank.feature_set.is_active( + &feature_set::include_loaded_accounts_data_size_in_fee_calculation::id(), + ), + ); + let (mut fee_payer_account, _slot) = bank + .rc + .accounts + .accounts_db + .load_with_fixed_root(&bank.ancestors, fee_payer) + .ok_or(TransactionError::AccountNotFound)?; + + validate_fee_payer( + fee_payer, + &mut fee_payer_account, + 0, + error_counters, + bank.rent_collector(), + fee, + ) + } + fn accumulate_execute_units_and_time(execute_timings: &ExecuteTimings) -> (u64, u64) { execute_timings.details.per_program_timings.values().fold( (0, 0), diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 840a2cf86..f8d99c779 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -1,5 +1,6 @@ use { super::{ + consumer::Consumer, forward_packet_batches_by_accounts::ForwardPacketBatchesByAccounts, immutable_deserialized_packet::ImmutableDeserializedPacket, latest_unprocessed_votes::{ @@ -16,6 +17,7 @@ use { }, itertools::Itertools, min_max_heap::MinMaxHeap, + solana_accounts_db::transaction_error_metrics::TransactionErrorMetrics, solana_measure::{measure, measure_us}, solana_runtime::bank::Bank, solana_sdk::{ @@ -136,6 +138,7 @@ pub struct ConsumeScannerPayload<'a> { pub sanitized_transactions: Vec, pub slot_metrics_tracker: &'a mut LeaderSlotMetricsTracker, pub message_hash_to_transaction: &'a mut HashMap, + pub error_counters: TransactionErrorMetrics, } fn consume_scan_should_process_packet( @@ -177,6 +180,27 @@ fn consume_scan_should_process_packet( return ProcessingDecision::Never; } + // Only check fee-payer if we can actually take locks + // We do not immediately discard on check lock failures here, + // because the priority guard requires that we always take locks + // except in the cases of discarding transactions (i.e. `Never`). + if payload.account_locks.check_locks(message) + && Consumer::check_fee_payer_unlocked(bank, message, &mut payload.error_counters) + .is_err() + { + payload + .message_hash_to_transaction + .remove(packet.message_hash()); + return ProcessingDecision::Never; + } + + // NOTE: + // This must be the last operation before adding the transaction to the + // sanitized_transactions vector. Otherwise, a transaction could + // be blocked by a transaction that did not take batch locks. This + // will lead to some transactions never being processed, and a + // mismatch in the priorty-queue and hash map sizes. + // // Always take locks during batch creation. // This prevents lower-priority transactions from taking locks // needed by higher-priority txs that were skipped by this check. @@ -213,6 +237,7 @@ where sanitized_transactions: Vec::with_capacity(UNPROCESSED_BUFFER_STEP_SIZE), slot_metrics_tracker, message_hash_to_transaction, + error_counters: TransactionErrorMetrics::default(), }; MultiIteratorScanner::new( packets, diff --git a/runtime/src/accounts/mod.rs b/runtime/src/accounts/mod.rs index ef801be65..26d68c3b8 100644 --- a/runtime/src/accounts/mod.rs +++ b/runtime/src/accounts/mod.rs @@ -479,7 +479,7 @@ fn accumulate_and_check_loaded_account_data_size( } } -fn validate_fee_payer( +pub fn validate_fee_payer( payer_address: &Pubkey, payer_account: &mut AccountSharedData, payer_index: IndexOfAccount,