diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 1f87be1ae86e44..55f18fbe969941 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -123,12 +123,14 @@ impl Accounts { } } + /// Return loaded addresses and the deactivation slot. + /// If the table hasn't been deactivated, the deactivation slot is `u64::MAX`. pub fn load_lookup_table_addresses( &self, ancestors: &Ancestors, address_table_lookup: &MessageAddressTableLookup, slot_hashes: &SlotHashes, - ) -> std::result::Result { + ) -> std::result::Result<(LoadedAddresses, Slot), AddressLookupError> { let table_account = self .accounts_db .load_with_fixed_root(ancestors, &address_table_lookup.account_key) @@ -140,18 +142,21 @@ impl Accounts { let lookup_table = AddressLookupTable::deserialize(table_account.data()) .map_err(|_ix_err| AddressLookupError::InvalidAccountData)?; - Ok(LoadedAddresses { - writable: lookup_table.lookup( - current_slot, - &address_table_lookup.writable_indexes, - slot_hashes, - )?, - readonly: lookup_table.lookup( - current_slot, - &address_table_lookup.readonly_indexes, - slot_hashes, - )?, - }) + Ok(( + LoadedAddresses { + writable: lookup_table.lookup( + current_slot, + &address_table_lookup.writable_indexes, + slot_hashes, + )?, + readonly: lookup_table.lookup( + current_slot, + &address_table_lookup.readonly_indexes, + slot_hashes, + )?, + }, + lookup_table.meta.deactivation_slot, + )) } else { Err(AddressLookupError::InvalidAccountOwner) } @@ -1045,10 +1050,13 @@ mod tests { &address_table_lookup, &SlotHashes::default(), ), - Ok(LoadedAddresses { - writable: vec![table_addresses[0]], - readonly: vec![table_addresses[1]], - }), + Ok(( + LoadedAddresses { + writable: vec![table_addresses[0]], + readonly: vec![table_addresses[1]], + }, + u64::MAX + )), ); } diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 57a4778d3204b3..7e1449c5c7e2f9 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -107,7 +107,7 @@ impl ConsumeWorker { let output = self.consumer.process_and_record_aged_transactions( bank, &work.transactions, - &work.max_age_slots, + &work.max_ages, ); self.metrics.update_for_consume(&output); @@ -700,7 +700,7 @@ mod tests { crate::banking_stage::{ committer::Committer, qos_service::QosService, - scheduler_messages::{TransactionBatchId, TransactionId}, + scheduler_messages::{MaxAge, TransactionBatchId, TransactionId}, tests::{create_slow_genesis_config, sanitize_transactions, simulate_poh}, }, crossbeam_channel::unbounded, @@ -714,10 +714,24 @@ mod tests { vote_sender_types::ReplayVoteReceiver, }, solana_sdk::{ - genesis_config::GenesisConfig, poh_config::PohConfig, pubkey::Pubkey, - signature::Keypair, system_transaction, + address_lookup_table::AddressLookupTableAccount, + clock::{Slot, MAX_PROCESSING_AGE}, + genesis_config::GenesisConfig, + message::{ + v0::{self, LoadedAddresses}, + SimpleAddressLoader, VersionedMessage, + }, + poh_config::PohConfig, + pubkey::Pubkey, + signature::Keypair, + signer::Signer, + system_instruction, system_transaction, + transaction::{ + MessageHash, SanitizedTransaction, TransactionError, VersionedTransaction, + }, }, std::{ + collections::HashSet, sync::{atomic::AtomicBool, RwLock}, thread::JoinHandle, }, @@ -748,6 +762,7 @@ mod tests { .. } = create_slow_genesis_config(10_000); let (bank, bank_forks) = Bank::new_no_wallclock_throttle_for_tests(&genesis_config); + let bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::new_unique(), 1)); let ledger_path = get_tmp_ledger_path_auto_delete!(); let blockstore = Blockstore::open(ledger_path.path()) @@ -826,17 +841,21 @@ mod tests { )]); let bid = TransactionBatchId::new(0); let id = TransactionId::new(0); + let max_age = MaxAge { + epoch_invalidation_slot: bank.slot(), + alt_invalidation_slot: bank.slot(), + }; let work = ConsumeWork { batch_id: bid, ids: vec![id], transactions, - max_age_slots: vec![bank.slot()], + max_ages: vec![max_age], }; consume_sender.send(work).unwrap(); let consumed = consumed_receiver.recv().unwrap(); assert_eq!(consumed.work.batch_id, bid); assert_eq!(consumed.work.ids, vec![id]); - assert_eq!(consumed.work.max_age_slots, vec![bank.slot()]); + assert_eq!(consumed.work.max_ages, vec![max_age]); assert_eq!(consumed.retryable_indexes, vec![0]); drop(test_frame); @@ -871,17 +890,21 @@ mod tests { )]); let bid = TransactionBatchId::new(0); let id = TransactionId::new(0); + let max_age = MaxAge { + epoch_invalidation_slot: bank.slot(), + alt_invalidation_slot: bank.slot(), + }; let work = ConsumeWork { batch_id: bid, ids: vec![id], transactions, - max_age_slots: vec![bank.slot()], + max_ages: vec![max_age], }; consume_sender.send(work).unwrap(); let consumed = consumed_receiver.recv().unwrap(); assert_eq!(consumed.work.batch_id, bid); assert_eq!(consumed.work.ids, vec![id]); - assert_eq!(consumed.work.max_age_slots, vec![bank.slot()]); + assert_eq!(consumed.work.max_ages, vec![max_age]); assert_eq!(consumed.retryable_indexes, Vec::::new()); drop(test_frame); @@ -917,19 +940,23 @@ mod tests { let bid = TransactionBatchId::new(0); let id1 = TransactionId::new(1); let id2 = TransactionId::new(0); + let max_age = MaxAge { + epoch_invalidation_slot: bank.slot(), + alt_invalidation_slot: bank.slot(), + }; consume_sender .send(ConsumeWork { batch_id: bid, ids: vec![id1, id2], transactions: txs, - max_age_slots: vec![bank.slot(), bank.slot()], + max_ages: vec![max_age, max_age], }) .unwrap(); let consumed = consumed_receiver.recv().unwrap(); assert_eq!(consumed.work.batch_id, bid); assert_eq!(consumed.work.ids, vec![id1, id2]); - assert_eq!(consumed.work.max_age_slots, vec![bank.slot(), bank.slot()]); + assert_eq!(consumed.work.max_ages, vec![max_age, max_age]); assert_eq!(consumed.retryable_indexes, vec![1]); // id2 is retryable since lock conflict drop(test_frame); @@ -974,12 +1001,16 @@ mod tests { let bid2 = TransactionBatchId::new(1); let id1 = TransactionId::new(1); let id2 = TransactionId::new(0); + let max_age = MaxAge { + epoch_invalidation_slot: bank.slot(), + alt_invalidation_slot: bank.slot(), + }; consume_sender .send(ConsumeWork { batch_id: bid1, ids: vec![id1], transactions: txs1, - max_age_slots: vec![bank.slot()], + max_ages: vec![max_age], }) .unwrap(); @@ -988,22 +1019,185 @@ mod tests { batch_id: bid2, ids: vec![id2], transactions: txs2, - max_age_slots: vec![bank.slot()], + max_ages: vec![max_age], }) .unwrap(); let consumed = consumed_receiver.recv().unwrap(); assert_eq!(consumed.work.batch_id, bid1); assert_eq!(consumed.work.ids, vec![id1]); - assert_eq!(consumed.work.max_age_slots, vec![bank.slot()]); + assert_eq!(consumed.work.max_ages, vec![max_age]); assert_eq!(consumed.retryable_indexes, Vec::::new()); let consumed = consumed_receiver.recv().unwrap(); assert_eq!(consumed.work.batch_id, bid2); assert_eq!(consumed.work.ids, vec![id2]); - assert_eq!(consumed.work.max_age_slots, vec![bank.slot()]); + assert_eq!(consumed.work.max_ages, vec![max_age]); assert_eq!(consumed.retryable_indexes, Vec::::new()); drop(test_frame); let _ = worker_thread.join().unwrap(); } + + #[test] + fn test_worker_ttl() { + let (test_frame, worker) = setup_test_frame(); + let TestFrame { + mint_keypair, + genesis_config, + bank, + poh_recorder, + consume_sender, + consumed_receiver, + .. + } = &test_frame; + let worker_thread = std::thread::spawn(move || worker.run()); + poh_recorder + .write() + .unwrap() + .set_bank_for_test(bank.clone()); + assert!(bank.slot() > 0); + + // No conflicts between transactions. Test 6 cases. + // 1. Epoch expiration, before slot => still succeeds due to resanitizing + // 2. Epoch expiration, on slot => succeeds normally + // 3. Epoch expiration, after slot => succeeds normally + // 4. ALT expiration, before slot => fails + // 5. ALT expiration, on slot => succeeds normally + // 6. ALT expiration, after slot => succeeds normally + let simple_transfer = || { + system_transaction::transfer( + &Keypair::new(), + &Pubkey::new_unique(), + 1, + genesis_config.hash(), + ) + }; + let simple_v0_transfer = || { + let payer = Keypair::new(); + let to_pubkey = Pubkey::new_unique(); + let loaded_addresses = LoadedAddresses { + writable: vec![to_pubkey], + readonly: vec![], + }; + let loader = SimpleAddressLoader::Enabled(loaded_addresses); + SanitizedTransaction::try_create( + VersionedTransaction::try_new( + VersionedMessage::V0( + v0::Message::try_compile( + &payer.pubkey(), + &[system_instruction::transfer(&payer.pubkey(), &to_pubkey, 1)], + &[AddressLookupTableAccount { + key: Pubkey::new_unique(), // will fail if using **bank** to lookup + addresses: vec![to_pubkey], + }], + genesis_config.hash(), + ) + .unwrap(), + ), + &[&payer], + ) + .unwrap(), + MessageHash::Compute, + None, + loader, + &HashSet::default(), + ) + .unwrap() + }; + + let mut txs = sanitize_transactions(vec![ + simple_transfer(), + simple_transfer(), + simple_transfer(), + ]); + txs.push(simple_v0_transfer()); + txs.push(simple_v0_transfer()); + txs.push(simple_v0_transfer()); + let sanitized_txs = txs.clone(); + + // Fund the keypairs. + for tx in &txs { + bank.process_transaction(&system_transaction::transfer( + mint_keypair, + &tx.message().account_keys()[0], + 2, + genesis_config.hash(), + )) + .unwrap(); + } + + consume_sender + .send(ConsumeWork { + batch_id: TransactionBatchId::new(1), + ids: vec![ + TransactionId::new(0), + TransactionId::new(1), + TransactionId::new(2), + TransactionId::new(3), + TransactionId::new(4), + TransactionId::new(5), + ], + transactions: txs, + max_ages: vec![ + MaxAge { + epoch_invalidation_slot: bank.slot() - 1, + alt_invalidation_slot: Slot::MAX, + }, + MaxAge { + epoch_invalidation_slot: bank.slot(), + alt_invalidation_slot: Slot::MAX, + }, + MaxAge { + epoch_invalidation_slot: bank.slot() + 1, + alt_invalidation_slot: Slot::MAX, + }, + MaxAge { + epoch_invalidation_slot: u64::MAX, + alt_invalidation_slot: bank.slot() - 1, + }, + MaxAge { + epoch_invalidation_slot: u64::MAX, + alt_invalidation_slot: bank.slot(), + }, + MaxAge { + epoch_invalidation_slot: u64::MAX, + alt_invalidation_slot: bank.slot() + 1, + }, + ], + }) + .unwrap(); + + let consumed = consumed_receiver.recv().unwrap(); + assert_eq!(consumed.retryable_indexes, Vec::::new()); + // all but one succeed. 6 for initial funding + assert_eq!(bank.transaction_count(), 6 + 5); + + let already_processed_results = bank + .check_transactions( + &sanitized_txs, + &vec![Ok(()); sanitized_txs.len()], + MAX_PROCESSING_AGE, + &mut TransactionErrorMetrics::default(), + ) + .into_iter() + .map(|r| match r { + Ok(_) => Ok(()), + Err(err) => Err(err), + }) + .collect::>(); + assert_eq!( + already_processed_results, + vec![ + Err(TransactionError::AlreadyProcessed), + Err(TransactionError::AlreadyProcessed), + Err(TransactionError::AlreadyProcessed), + Ok(()), // <--- this transaction was not processed + Err(TransactionError::AlreadyProcessed), + Err(TransactionError::AlreadyProcessed) + ] + ); + + drop(test_frame); + let _ = worker_thread.join().unwrap(); + } } diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 3493a5f3a85408..4798991e9f206b 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -5,6 +5,7 @@ use { leader_slot_metrics::{LeaderSlotMetricsTracker, ProcessTransactionsSummary}, leader_slot_timing_metrics::LeaderExecuteAndCommitTimings, qos_service::QosService, + scheduler_messages::MaxAge, unprocessed_transaction_storage::{ConsumeScannerPayload, UnprocessedTransactionStorage}, BankingStageStats, }, @@ -23,12 +24,12 @@ use { transaction_batch::TransactionBatch, }, solana_sdk::{ - clock::{Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE}, + clock::{FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE}, feature_set, message::SanitizedMessage, saturating_add_assign, timing::timestamp, - transaction::{self, AddressLoader, SanitizedTransaction, TransactionError}, + transaction::{self, SanitizedTransaction, TransactionError}, }, solana_svm::{ account_loader::{validate_fee_payer, TransactionCheckResult}, @@ -440,14 +441,15 @@ impl Consumer { &self, bank: &Arc, txs: &[SanitizedTransaction], - max_slot_ages: &[Slot], + max_ages: &[MaxAge], ) -> ProcessTransactionBatchOutput { // Verify pre-compiles. // Need to filter out transactions since they were sanitized earlier. // This means that the transaction may cross and epoch boundary (not allowed), // or account lookup tables may have been closed. - let pre_results = txs.iter().zip(max_slot_ages).map(|(tx, max_slot_age)| { - if *max_slot_age < bank.slot() { + let pre_results = txs.iter().zip(max_ages).map(|(tx, max_age)| { + if bank.slot() > max_age.epoch_invalidation_slot { + // Epoch has rolled over. Need to fully re-verify the transaction. // Pre-compiles are verified here. // Attempt re-sanitization after epoch-cross. // Re-sanitized transaction should be equal to the original transaction, @@ -459,15 +461,23 @@ impl Consumer { return Err(TransactionError::ResanitizationNeeded); } } else { + if bank.slot() > max_age.alt_invalidation_slot { + // The address table lookup **may** have expired, but the + // expiration is not guaranteed since there may have been + // skipped slot. + // If the addresses still resolve here, then the transaction is still + // valid, and we can continue with processing. + // If they do not, then the ATL has expired and the transaction + // can be dropped. + let (_addresses, _deactivation_slot) = bank.load_addresses_from_ref( + tx.message().message_address_table_lookups().iter(), + )?; + } + // Verify pre-compiles. tx.verify_precompiles(&bank.feature_set)?; - // Any transaction executed between sanitization time and now may have closed the lookup table(s). - // Above re-sanitization already loads addresses, so don't need to re-check in that case. - let lookup_tables = tx.message().message_address_table_lookups(); - if !lookup_tables.is_empty() { - bank.load_addresses(lookup_tables)?; - } } + Ok(()) }); self.process_and_record_transactions_with_pre_results(bank, txs, 0, pre_results) diff --git a/core/src/banking_stage/immutable_deserialized_packet.rs b/core/src/banking_stage/immutable_deserialized_packet.rs index cb4561d50d8f08..fff7747e64e9b3 100644 --- a/core/src/banking_stage/immutable_deserialized_packet.rs +++ b/core/src/banking_stage/immutable_deserialized_packet.rs @@ -1,18 +1,19 @@ use { super::packet_filter::PacketFilterFailure, solana_perf::packet::Packet, - solana_runtime::compute_budget_details::{ComputeBudgetDetails, GetComputeBudgetDetails}, + solana_runtime::{ + bank::Bank, + compute_budget_details::{ComputeBudgetDetails, GetComputeBudgetDetails}, + }, solana_sdk::{ + clock::Slot, hash::Hash, - message::Message, + message::{v0::LoadedAddresses, AddressLoaderError, Message, SimpleAddressLoader}, pubkey::Pubkey, sanitize::SanitizeError, short_vec::decode_shortu16_len, signature::Signature, - transaction::{ - AddressLoader, SanitizedTransaction, SanitizedVersionedTransaction, - VersionedTransaction, - }, + transaction::{SanitizedTransaction, SanitizedVersionedTransaction, VersionedTransaction}, }, std::{cmp::Ordering, collections::HashSet, mem::size_of}, thiserror::Error, @@ -103,15 +104,22 @@ impl ImmutableDeserializedPacket { // This function deserializes packets into transactions, computes the blake3 hash of transaction // messages. + // Additionally, this returns the minimum deactivation slot of the resolved addresses. pub fn build_sanitized_transaction( &self, votes_only: bool, - address_loader: impl AddressLoader, + bank: &Bank, reserved_account_keys: &HashSet, - ) -> Option { + ) -> Option<(SanitizedTransaction, Slot)> { if votes_only && !self.is_simple_vote() { return None; } + + // Resolve the lookup addresses and retrieve the min deactivation slot + let (loaded_addresses, deactivation_slot) = + Self::resolve_addresses_with_deactivation(self.transaction(), bank).ok()?; + let address_loader = SimpleAddressLoader::Enabled(loaded_addresses); + let tx = SanitizedTransaction::try_new( self.transaction().clone(), *self.message_hash(), @@ -120,7 +128,19 @@ impl ImmutableDeserializedPacket { reserved_account_keys, ) .ok()?; - Some(tx) + Some((tx, deactivation_slot)) + } + + fn resolve_addresses_with_deactivation( + transaction: &SanitizedVersionedTransaction, + bank: &Bank, + ) -> Result<(LoadedAddresses, Slot), AddressLoaderError> { + let Some(address_table_lookups) = transaction.get_message().message.address_table_lookups() + else { + return Ok((LoadedAddresses::default(), Slot::MAX)); + }; + + bank.load_addresses_from_ref(address_table_lookups.iter()) } } diff --git a/core/src/banking_stage/latest_unprocessed_votes.rs b/core/src/banking_stage/latest_unprocessed_votes.rs index fc6036bee67701..3eaeea34ffeaf5 100644 --- a/core/src/banking_stage/latest_unprocessed_votes.rs +++ b/core/src/banking_stage/latest_unprocessed_votes.rs @@ -362,8 +362,8 @@ impl LatestUnprocessedVotes { let mut vote = lock.write().unwrap(); if !vote.is_vote_taken() && !vote.is_forwarded() { let deserialized_vote_packet = vote.vote.as_ref().unwrap().clone(); - if let Some(sanitized_vote_transaction) = deserialized_vote_packet - .build_sanitized_transaction( + if let Some((sanitized_vote_transaction, _deactivation_slot)) = + deserialized_vote_packet.build_sanitized_transaction( bank.vote_only_bank(), bank.as_ref(), bank.get_reserved_account_keys(), diff --git a/core/src/banking_stage/scheduler_messages.rs b/core/src/banking_stage/scheduler_messages.rs index 92181e2abf9655..3c5da788382726 100644 --- a/core/src/banking_stage/scheduler_messages.rs +++ b/core/src/banking_stage/scheduler_messages.rs @@ -36,13 +36,19 @@ impl Display for TransactionId { } } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct MaxAge { + pub epoch_invalidation_slot: Slot, + pub alt_invalidation_slot: Slot, +} + /// Message: [Scheduler -> Worker] /// Transactions to be consumed (i.e. executed, recorded, and committed) pub struct ConsumeWork { pub batch_id: TransactionBatchId, pub ids: Vec, pub transactions: Vec, - pub max_age_slots: Vec, + pub max_ages: Vec, } /// Message: [Scheduler -> Worker] diff --git a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs index 045d2cca1d8dba..b31a3d60f59311 100644 --- a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs +++ b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs @@ -9,7 +9,9 @@ use { crate::banking_stage::{ consumer::TARGET_NUM_TRANSACTIONS_PER_BATCH, read_write_account_set::ReadWriteAccountSet, - scheduler_messages::{ConsumeWork, FinishedConsumeWork, TransactionBatchId, TransactionId}, + scheduler_messages::{ + ConsumeWork, FinishedConsumeWork, MaxAge, TransactionBatchId, TransactionId, + }, transaction_scheduler::{ transaction_priority_id::TransactionPriorityId, transaction_state::TransactionState, }, @@ -19,10 +21,7 @@ use { prio_graph::{AccessKind, PrioGraph}, solana_cost_model::block_cost_limits::MAX_BLOCK_UNITS, solana_measure::measure_us, - solana_sdk::{ - pubkey::Pubkey, saturating_add_assign, slot_history::Slot, - transaction::SanitizedTransaction, - }, + solana_sdk::{pubkey::Pubkey, saturating_add_assign, transaction::SanitizedTransaction}, }; pub(crate) struct PrioGraphScheduler { @@ -202,13 +201,13 @@ impl PrioGraphScheduler { Ok(TransactionSchedulingInfo { thread_id, transaction, - max_age_slot, + max_age, cost, }) => { saturating_add_assign!(num_scheduled, 1); batches.transactions[thread_id].push(transaction); batches.ids[thread_id].push(id.id); - batches.max_age_slots[thread_id].push(max_age_slot); + batches.max_ages[thread_id].push(max_age); saturating_add_assign!(batches.total_cus[thread_id], cost); // If target batch size is reached, send only this batch. @@ -309,7 +308,7 @@ impl PrioGraphScheduler { batch_id, ids, transactions, - max_age_slots, + max_ages, }, retryable_indexes, }) => { @@ -321,8 +320,8 @@ impl PrioGraphScheduler { // Retryable transactions should be inserted back into the container let mut retryable_iter = retryable_indexes.into_iter().peekable(); - for (index, (id, transaction, max_age_slot)) in - izip!(ids, transactions, max_age_slots).enumerate() + for (index, (id, transaction, max_age)) in + izip!(ids, transactions, max_ages).enumerate() { if let Some(retryable_index) = retryable_iter.peek() { if *retryable_index == index { @@ -330,7 +329,7 @@ impl PrioGraphScheduler { id, SanitizedTransactionTTL { transaction, - max_age_slot, + max_age, }, ); retryable_iter.next(); @@ -392,7 +391,7 @@ impl PrioGraphScheduler { return Ok(0); } - let (ids, transactions, max_age_slots, total_cus) = batches.take_batch(thread_index); + let (ids, transactions, max_ages, total_cus) = batches.take_batch(thread_index); let batch_id = self .in_flight_tracker @@ -403,7 +402,7 @@ impl PrioGraphScheduler { batch_id, ids, transactions, - max_age_slots, + max_ages, }; self.consume_work_senders[thread_index] .send(work) @@ -477,7 +476,7 @@ pub(crate) struct SchedulingSummary { struct Batches { ids: Vec>, transactions: Vec>, - max_age_slots: Vec>, + max_ages: Vec>, total_cus: Vec, } @@ -486,7 +485,7 @@ impl Batches { Self { ids: vec![Vec::with_capacity(TARGET_NUM_TRANSACTIONS_PER_BATCH); num_threads], transactions: vec![Vec::with_capacity(TARGET_NUM_TRANSACTIONS_PER_BATCH); num_threads], - max_age_slots: vec![Vec::with_capacity(TARGET_NUM_TRANSACTIONS_PER_BATCH); num_threads], + max_ages: vec![Vec::with_capacity(TARGET_NUM_TRANSACTIONS_PER_BATCH); num_threads], total_cus: vec![0; num_threads], } } @@ -497,7 +496,7 @@ impl Batches { ) -> ( Vec, Vec, - Vec, + Vec, u64, ) { ( @@ -510,7 +509,7 @@ impl Batches { Vec::with_capacity(TARGET_NUM_TRANSACTIONS_PER_BATCH), ), core::mem::replace( - &mut self.max_age_slots[thread_id], + &mut self.max_ages[thread_id], Vec::with_capacity(TARGET_NUM_TRANSACTIONS_PER_BATCH), ), core::mem::replace(&mut self.total_cus[thread_id], 0), @@ -522,7 +521,7 @@ impl Batches { struct TransactionSchedulingInfo { thread_id: ThreadId, transaction: SanitizedTransaction, - max_age_slot: Slot, + max_age: MaxAge, cost: u64, } @@ -573,7 +572,7 @@ fn try_schedule_transaction( Ok(TransactionSchedulingInfo { thread_id, transaction: sanitized_transaction_ttl.transaction, - max_age_slot: sanitized_transaction_ttl.max_age_slot, + max_age: sanitized_transaction_ttl.max_age, cost, }) } @@ -589,8 +588,8 @@ mod tests { crossbeam_channel::{unbounded, Receiver}, itertools::Itertools, solana_sdk::{ - compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, packet::Packet, - pubkey::Pubkey, signature::Keypair, signer::Signer, system_instruction, + clock::Slot, compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, + packet::Packet, pubkey::Pubkey, signature::Keypair, signer::Signer, system_instruction, transaction::Transaction, }, std::{borrow::Borrow, sync::Arc}, @@ -676,7 +675,10 @@ mod tests { ); let transaction_ttl = SanitizedTransactionTTL { transaction, - max_age_slot: Slot::MAX, + max_age: MaxAge { + epoch_invalidation_slot: Slot::MAX, + alt_invalidation_slot: Slot::MAX, + }, }; const TEST_TRANSACTION_COST: u64 = 5000; container.insert_new_transaction( diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index 1092e2e012925e..08aaa6ac9c5daf 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -19,6 +19,7 @@ use { forwarder::Forwarder, immutable_deserialized_packet::ImmutableDeserializedPacket, packet_deserializer::PacketDeserializer, + scheduler_messages::MaxAge, ForwardOption, TOTAL_BUFFERED_PACKETS, }, crossbeam_channel::RecvTimeoutError, @@ -28,7 +29,8 @@ use { solana_runtime::{bank::Bank, bank_forks::BankForks}, solana_sdk::{ self, - clock::{FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE}, + address_lookup_table::state::estimate_last_valid_slot, + clock::{Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE}, fee::FeeBudgetLimits, saturating_add_assign, transaction::SanitizedTransaction, @@ -497,19 +499,29 @@ impl SchedulerController { // Convert to Arcs let packets: Vec<_> = packets.into_iter().map(Arc::new).collect(); // Sanitize packets, generate IDs, and insert into the container. - let bank = self.bank_forks.read().unwrap().working_bank(); - let last_slot_in_epoch = bank.epoch_schedule().get_last_slot_in_epoch(bank.epoch()); - let transaction_account_lock_limit = bank.get_transaction_account_lock_limit(); - let vote_only = bank.vote_only_bank(); + let (root_bank, working_bank) = { + let bank_forks = self.bank_forks.read().unwrap(); + let root_bank = bank_forks.root_bank(); + let working_bank = bank_forks.working_bank(); + (root_bank, working_bank) + }; + let alt_resolved_slot = root_bank.slot(); + let last_slot_in_epoch = working_bank + .epoch_schedule() + .get_last_slot_in_epoch(working_bank.epoch()); + let transaction_account_lock_limit = working_bank.get_transaction_account_lock_limit(); + let vote_only = working_bank.vote_only_bank(); const CHUNK_SIZE: usize = 128; let lock_results: [_; CHUNK_SIZE] = core::array::from_fn(|_| Ok(())); + let mut error_counts = TransactionErrorMetrics::default(); for chunk in packets.chunks(CHUNK_SIZE) { let mut post_sanitization_count: usize = 0; let mut arc_packets = Vec::with_capacity(chunk.len()); let mut transactions = Vec::with_capacity(chunk.len()); + let mut max_ages = Vec::with_capacity(chunk.len()); let mut fee_budget_limits_vec = Vec::with_capacity(chunk.len()); chunk @@ -518,31 +530,43 @@ impl SchedulerController { packet .build_sanitized_transaction( vote_only, - bank.as_ref(), - bank.get_reserved_account_keys(), + root_bank.as_ref(), + working_bank.get_reserved_account_keys(), ) - .map(|tx| (packet.clone(), tx)) + .map(|(tx, deactivation_slot)| (packet.clone(), tx, deactivation_slot)) }) .inspect(|_| saturating_add_assign!(post_sanitization_count, 1)) - .filter(|(_packet, tx)| { + .filter(|(_packet, tx, _deactivation_slot)| { SanitizedTransaction::validate_account_locks( tx.message(), transaction_account_lock_limit, ) .is_ok() }) - .filter_map(|(packet, tx)| { + .filter_map(|(packet, tx, deactivation_slot)| { process_compute_budget_instructions(tx.message().program_instructions_iter()) - .map(|compute_budget| (packet, tx, compute_budget.into())) + .map(|compute_budget| { + (packet, tx, deactivation_slot, compute_budget.into()) + }) .ok() }) - .for_each(|(packet, tx, fee_budget_limits)| { + .for_each(|(packet, tx, deactivation_slot, fee_budget_limits)| { arc_packets.push(packet); transactions.push(tx); + max_ages.push(calculate_max_age( + last_slot_in_epoch, + deactivation_slot, + alt_resolved_slot, + )); fee_budget_limits_vec.push(fee_budget_limits); }); - let check_results = bank.check_transactions( + let check_results: Vec< + Result< + solana_svm::account_loader::CheckedTransactionDetails, + solana_sdk::transaction::TransactionError, + >, + > = working_bank.check_transactions( &transactions, &lock_results[..transactions.len()], MAX_PROCESSING_AGE, @@ -553,9 +577,10 @@ impl SchedulerController { let mut post_transaction_check_count: usize = 0; let mut num_dropped_on_capacity: usize = 0; let mut num_buffered: usize = 0; - for (((packet, transaction), fee_budget_limits), _) in arc_packets + for ((((packet, transaction), max_age), fee_budget_limits), _) in arc_packets .into_iter() .zip(transactions) + .zip(max_ages) .zip(fee_budget_limits_vec) .zip(check_results) .filter(|(_, check_result)| check_result.is_ok()) @@ -563,11 +588,14 @@ impl SchedulerController { saturating_add_assign!(post_transaction_check_count, 1); let transaction_id = self.transaction_id_generator.next(); - let (priority, cost) = - Self::calculate_priority_and_cost(&transaction, &fee_budget_limits, &bank); + let (priority, cost) = Self::calculate_priority_and_cost( + &transaction, + &fee_budget_limits, + &working_bank, + ); let transaction_ttl = SanitizedTransactionTTL { transaction, - max_age_slot: last_slot_in_epoch, + max_age, }; if self.container.insert_new_transaction( @@ -652,6 +680,34 @@ impl SchedulerController { } } +/// Given the last slot in the epoch, the minimum deactivation slot, +/// and the current slot, return the `MaxAge` that should be used for +/// the transaction. This is used to determine the maximum slot that a +/// transaction will be considered valid for, without re-resolving addresses +/// or resanitizing. +/// +/// This function considers the deactivation period of Address Table +/// accounts. If the deactivation period runs past the end of the epoch, +/// then the transaction is considered valid until the end of the epoch. +/// Otherwise, the transaction is considered valid until the deactivation +/// period. +/// +/// Since the deactivation period technically uses blocks rather than +/// slots, the value used here is the lower-bound on the deactivation +/// period, i.e. the transaction's address lookups are valid until +/// AT LEAST this slot. +fn calculate_max_age( + last_slot_in_epoch: Slot, + deactivation_slot: Slot, + current_slot: Slot, +) -> MaxAge { + let alt_min_expire_slot = estimate_last_valid_slot(deactivation_slot.min(current_slot)); + MaxAge { + epoch_invalidation_slot: last_slot_in_epoch, + alt_invalidation_slot: alt_min_expire_slot, + } +} + #[cfg(test)] mod tests { use { @@ -822,7 +878,7 @@ mod tests { batch_id: TransactionBatchId::new(0), ids: vec![], transactions: vec![], - max_age_slots: vec![], + max_ages: vec![], }, retryable_indexes: vec![], }) @@ -1153,4 +1209,29 @@ mod tests { .collect_vec(); assert_eq!(message_hashes, vec![&tx1_hash]); } + + #[test] + fn test_calculate_max_age() { + let current_slot = 100; + let last_slot_in_epoch = 1000; + + // ALT deactivation slot is delayed + assert_eq!( + calculate_max_age(last_slot_in_epoch, current_slot - 1, current_slot), + MaxAge { + epoch_invalidation_slot: last_slot_in_epoch, + alt_invalidation_slot: current_slot - 1 + + solana_sdk::slot_hashes::get_entries() as u64, + } + ); + + // no deactivation slot + assert_eq!( + calculate_max_age(last_slot_in_epoch, u64::MAX, current_slot), + MaxAge { + epoch_invalidation_slot: last_slot_in_epoch, + alt_invalidation_slot: current_slot + solana_sdk::slot_hashes::get_entries() as u64, + } + ); + } } diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state.rs b/core/src/banking_stage/transaction_scheduler/transaction_state.rs index 85af8217309e93..efb59be1b8b5b5 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state.rs @@ -1,13 +1,15 @@ use { - crate::banking_stage::immutable_deserialized_packet::ImmutableDeserializedPacket, - solana_sdk::{clock::Slot, transaction::SanitizedTransaction}, + crate::banking_stage::{ + immutable_deserialized_packet::ImmutableDeserializedPacket, scheduler_messages::MaxAge, + }, + solana_sdk::transaction::SanitizedTransaction, std::sync::Arc, }; /// Simple wrapper type to tie a sanitized transaction to max age slot. pub(crate) struct SanitizedTransactionTTL { pub(crate) transaction: SanitizedTransaction, - pub(crate) max_age_slot: Slot, + pub(crate) max_age: MaxAge, } /// TransactionState is used to track the state of a transaction in the transaction scheduler @@ -207,8 +209,9 @@ mod tests { use { super::*, solana_sdk::{ - compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, packet::Packet, - signature::Keypair, signer::Signer, system_instruction, transaction::Transaction, + clock::Slot, compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, + packet::Packet, signature::Keypair, signer::Signer, system_instruction, + transaction::Transaction, }, }; @@ -230,7 +233,10 @@ mod tests { ); let transaction_ttl = SanitizedTransactionTTL { transaction: SanitizedTransaction::from_transaction_for_tests(tx), - max_age_slot: Slot::MAX, + max_age: MaxAge { + epoch_invalidation_slot: Slot::MAX, + alt_invalidation_slot: Slot::MAX, + }, }; const TEST_TRANSACTION_COST: u64 = 5000; TransactionState::new( @@ -271,11 +277,11 @@ mod tests { // Manually clone `SanitizedTransactionTTL` let SanitizedTransactionTTL { transaction, - max_age_slot, + max_age, } = transaction_state.transaction_ttl(); let transaction_ttl = SanitizedTransactionTTL { transaction: transaction.clone(), - max_age_slot: *max_age_slot, + max_age: *max_age, }; transaction_state.transition_to_unprocessed(transaction_ttl); // invalid transition } @@ -321,7 +327,13 @@ mod tests { transaction_state, TransactionState::Unprocessed { .. } )); - assert_eq!(transaction_ttl.max_age_slot, Slot::MAX); + assert_eq!( + transaction_ttl.max_age, + MaxAge { + epoch_invalidation_slot: Slot::MAX, + alt_invalidation_slot: Slot::MAX, + } + ); let _ = transaction_state.transition_to_pending(); assert!(matches!( @@ -339,7 +351,13 @@ mod tests { transaction_state, TransactionState::Unprocessed { .. } )); - assert_eq!(transaction_ttl.max_age_slot, Slot::MAX); + assert_eq!( + transaction_ttl.max_age, + MaxAge { + epoch_invalidation_slot: Slot::MAX, + alt_invalidation_slot: Slot::MAX, + } + ); // ensure transaction_ttl is not lost through state transitions let transaction_ttl = transaction_state.transition_to_pending(); @@ -354,6 +372,12 @@ mod tests { transaction_state, TransactionState::Unprocessed { .. } )); - assert_eq!(transaction_ttl.max_age_slot, Slot::MAX); + assert_eq!( + transaction_ttl.max_age, + MaxAge { + epoch_invalidation_slot: Slot::MAX, + alt_invalidation_slot: Slot::MAX, + } + ); } } diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs index 3f804f662652ab..710b140edea8ab 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs @@ -153,6 +153,7 @@ impl TransactionStateContainer { mod tests { use { super::*, + crate::banking_stage::scheduler_messages::MaxAge, solana_sdk::{ compute_budget::ComputeBudgetInstruction, hash::Hash, @@ -198,7 +199,10 @@ mod tests { ); let transaction_ttl = SanitizedTransactionTTL { transaction: tx, - max_age_slot: Slot::MAX, + max_age: MaxAge { + epoch_invalidation_slot: Slot::MAX, + alt_invalidation_slot: Slot::MAX, + }, }; const TEST_TRANSACTION_COST: u64 = 5000; (transaction_ttl, packet, priority, TEST_TRANSACTION_COST) diff --git a/core/src/banking_stage/unprocessed_packet_batches.rs b/core/src/banking_stage/unprocessed_packet_batches.rs index 2bec44dbd0ea5e..3eb20a9861bee7 100644 --- a/core/src/banking_stage/unprocessed_packet_batches.rs +++ b/core/src/banking_stage/unprocessed_packet_batches.rs @@ -307,13 +307,14 @@ mod tests { use { super::*, solana_perf::packet::PacketFlags, + solana_runtime::bank::Bank, solana_sdk::{ compute_budget::ComputeBudgetInstruction, message::Message, reserved_account_keys::ReservedAccountKeys, signature::{Keypair, Signer}, system_instruction, system_transaction, - transaction::{SimpleAddressLoader, Transaction}, + transaction::Transaction, }, solana_vote_program::vote_transaction, }; @@ -476,6 +477,7 @@ mod tests { &keypair, None, ); + let bank = Bank::default_for_tests(); // packets with no votes { @@ -487,7 +489,7 @@ mod tests { let txs = packet_vector.iter().filter_map(|tx| { tx.immutable_section().build_sanitized_transaction( votes_only, - SimpleAddressLoader::Disabled, + &bank, &ReservedAccountKeys::empty_key_set(), ) }); @@ -497,7 +499,7 @@ mod tests { let txs = packet_vector.iter().filter_map(|tx| { tx.immutable_section().build_sanitized_transaction( votes_only, - SimpleAddressLoader::Disabled, + &bank, &ReservedAccountKeys::empty_key_set(), ) }); @@ -516,7 +518,7 @@ mod tests { let txs = packet_vector.iter().filter_map(|tx| { tx.immutable_section().build_sanitized_transaction( votes_only, - SimpleAddressLoader::Disabled, + &bank, &ReservedAccountKeys::empty_key_set(), ) }); @@ -526,7 +528,7 @@ mod tests { let txs = packet_vector.iter().filter_map(|tx| { tx.immutable_section().build_sanitized_transaction( votes_only, - SimpleAddressLoader::Disabled, + &bank, &ReservedAccountKeys::empty_key_set(), ) }); @@ -545,7 +547,7 @@ mod tests { let txs = packet_vector.iter().filter_map(|tx| { tx.immutable_section().build_sanitized_transaction( votes_only, - SimpleAddressLoader::Disabled, + &bank, &ReservedAccountKeys::empty_key_set(), ) }); @@ -555,7 +557,7 @@ mod tests { let txs = packet_vector.iter().filter_map(|tx| { tx.immutable_section().build_sanitized_transaction( votes_only, - SimpleAddressLoader::Disabled, + &bank, &ReservedAccountKeys::empty_key_set(), ) }); diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 7c019993aed012..862ae97b4af865 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -152,13 +152,15 @@ fn consume_scan_should_process_packet( return ProcessingDecision::Now; } - // Try to sanitize the packet + // Try to sanitize the packet. Ignore deactivation slot since we are + // immediately attempting to process the transaction. let (maybe_sanitized_transaction, sanitization_time_us) = measure_us!(packet .build_sanitized_transaction( bank.vote_only_bank(), bank, bank.get_reserved_account_keys(), - )); + ) + .map(|(tx, _deactivation_slot)| tx)); payload .slot_metrics_tracker @@ -808,7 +810,7 @@ impl ThreadLocalUnprocessedPackets { bank, bank.get_reserved_account_keys(), ) - .map(|transaction| (transaction, packet_index)) + .map(|(transaction, _deactivation_slot)| (transaction, packet_index)) }) .unzip(); diff --git a/runtime/src/bank/address_lookup_table.rs b/runtime/src/bank/address_lookup_table.rs index 344f1e8bdf09aa..d8aa565d5f16c1 100644 --- a/runtime/src/bank/address_lookup_table.rs +++ b/runtime/src/bank/address_lookup_table.rs @@ -2,6 +2,7 @@ use { super::Bank, solana_sdk::{ address_lookup_table::error::AddressLookupError, + clock::Slot, message::{ v0::{LoadedAddresses, MessageAddressTableLookup}, AddressLoaderError, @@ -26,14 +27,26 @@ impl AddressLoader for &Bank { self, address_table_lookups: &[MessageAddressTableLookup], ) -> Result { + self.load_addresses_from_ref(address_table_lookups.iter()) + .map(|(loaded_addresses, _deactivation_slot)| loaded_addresses) + } +} + +impl Bank { + /// Load addresses from an iterator of `SVMMessageAddressTableLookup`, + /// additionally returning the minimum deactivation slot across all referenced ALTs + pub fn load_addresses_from_ref<'a>( + &self, + address_table_lookups: impl Iterator, + ) -> Result<(LoadedAddresses, Slot), AddressLoaderError> { let slot_hashes = self .transaction_processor .sysvar_cache() .get_slot_hashes() .map_err(|_| AddressLoaderError::SlotHashesSysvarNotFound)?; - address_table_lookups - .iter() + let mut deactivation_slot = u64::MAX; + let loaded_addresses = address_table_lookups .map(|address_table_lookup| { self.rc .accounts @@ -42,8 +55,14 @@ impl AddressLoader for &Bank { address_table_lookup, &slot_hashes, ) + .map(|(loaded_addresses, table_deactivation_slot)| { + deactivation_slot = deactivation_slot.min(table_deactivation_slot); + loaded_addresses + }) .map_err(into_address_loader_error) }) - .collect::>() + .collect::>()?; + + Ok((loaded_addresses, deactivation_slot)) } } diff --git a/sdk/program/src/address_lookup_table/state.rs b/sdk/program/src/address_lookup_table/state.rs index df564f78fe2577..63cd8dacd7f9be 100644 --- a/sdk/program/src/address_lookup_table/state.rs +++ b/sdk/program/src/address_lookup_table/state.rs @@ -1,6 +1,7 @@ #[cfg(feature = "frozen-abi")] use solana_frozen_abi_macro::{AbiEnumVisitor, AbiExample}; use { + crate::slot_hashes::get_entries, serde_derive::{Deserialize, Serialize}, solana_program::{ address_lookup_table::error::AddressLookupError, @@ -12,6 +13,19 @@ use { std::borrow::Cow, }; +/// The lookup table may be in a deactivating state until +/// the `deactivation_slot`` is no longer "recent". +/// This function returns a conservative estimate for the +/// last block that the table may be used for lookups. +/// This estimate may be incorrect due to skipped blocks, +/// however, if the current slot is lower than the returned +/// value, the table is guaranteed to still be in the +/// deactivating state. +#[inline] +pub fn estimate_last_valid_slot(deactivation_slot: Slot) -> Slot { + deactivation_slot.saturating_add(get_entries() as Slot) +} + /// The maximum number of addresses that a lookup table can hold pub const LOOKUP_TABLE_MAX_ADDRESSES: usize = 256;