Skip to content

Commit

Permalink
earlier fee payer validation (solana-labs#34666)
Browse files Browse the repository at this point in the history
  • Loading branch information
apfitzge authored Jan 12, 2024
1 parent bc13642 commit be5337a
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 6 deletions.
63 changes: 58 additions & 5 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand Down
25 changes: 25 additions & 0 deletions core/src/banking_stage/unprocessed_transaction_storage.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use {
super::{
consumer::Consumer,
forward_packet_batches_by_accounts::ForwardPacketBatchesByAccounts,
immutable_deserialized_packet::ImmutableDeserializedPacket,
latest_unprocessed_votes::{
Expand All @@ -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::{
Expand Down Expand Up @@ -136,6 +138,7 @@ pub struct ConsumeScannerPayload<'a> {
pub sanitized_transactions: Vec<SanitizedTransaction>,
pub slot_metrics_tracker: &'a mut LeaderSlotMetricsTracker,
pub message_hash_to_transaction: &'a mut HashMap<Hash, DeserializedPacket>,
pub error_counters: TransactionErrorMetrics,
}

fn consume_scan_should_process_packet(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/accounts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit be5337a

Please sign in to comment.