From 2247916d54520f7b7ee790cbfbba8f6efb9048ad Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Fri, 5 Jan 2024 18:23:33 -0800 Subject: [PATCH 01/42] output txn signature for debug purpose trying txn mask matching output txn to figure out why txn is not exactly matched Use 62 and 61 portion track fetch performance using random txn mask track sigverify performance using random txn mask track banking stage performance using random txn mask adding missing cargo lock file add debug messages Revert "add debug messages" This reverts commit 96aead5cbc4acb5b2fc9d8a37fcd506c73ddf552. fixed some clippy issues check-crate issue Fix a clippy issue Fix a clippy issue debug why txns in banking stage shows fewer performance tracking points debug why txns in banking stage shows fewer performance tracking points debug why txns in banking stage shows fewer performance tracking points debug why txns in banking stage shows fewer performance tracking points get higher PPS for testing purpose more debug messages on why txn is skipped display if tracer packet in log add debug before calling processing_function debug at the initial of banking stage track if a txn is forwarded dependency order missing cargo file clean up debug messages Do not use TRACER_PACKET, use its own bit rename some functions addressed some comments from Trent Update core/src/banking_stage/immutable_deserialized_packet.rs Co-authored-by: Trent Nelson addressed some comments from Trent Do not use binary_search, do simple compare in one loop --- Cargo.lock | 16 ++++++ Cargo.toml | 2 + core/Cargo.toml | 1 + core/src/banking_stage/consumer.rs | 27 ++++++++++ .../immutable_deserialized_packet.rs | 13 ++++- .../unprocessed_transaction_storage.rs | 1 + core/src/sigverify_stage.rs | 29 +++++++++- programs/sbf/Cargo.lock | 16 ++++++ sdk/src/packet.rs | 13 +++++ sdk/src/transaction/versioned/sanitized.rs | 4 ++ streamer/Cargo.toml | 1 + streamer/src/nonblocking/quic.rs | 42 ++++++++++++++- transaction-metrics-tracker/Cargo.toml | 25 +++++++++ transaction-metrics-tracker/src/lib.rs | 54 +++++++++++++++++++ 14 files changed, 240 insertions(+), 4 deletions(-) create mode 100644 transaction-metrics-tracker/Cargo.toml create mode 100644 transaction-metrics-tracker/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index ce428b22a3283b..604c4ef598c029 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6011,6 +6011,7 @@ dependencies = [ "solana-streamer", "solana-svm", "solana-tpu-client", + "solana-transaction-metrics-tracker", "solana-transaction-status", "solana-turbine", "solana-unified-scheduler-pool", @@ -7279,6 +7280,7 @@ dependencies = [ "solana-metrics", "solana-perf", "solana-sdk", + "solana-transaction-metrics-tracker", "thiserror", "tokio", "x509-parser", @@ -7445,6 +7447,20 @@ dependencies = [ "solana-version", ] +[[package]] +name = "solana-transaction-metrics-tracker" +version = "1.19.0" +dependencies = [ + "Inflector", + "base64 0.21.7", + "bincode", + "lazy_static", + "log", + "rand 0.8.5", + "solana-perf", + "solana-sdk", +] + [[package]] name = "solana-transaction-status" version = "2.0.0" diff --git a/Cargo.toml b/Cargo.toml index 6cbd56762fbfe8..f50a7142821044 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -106,6 +106,7 @@ members = [ "tokens", "tpu-client", "transaction-dos", + "transaction-metrics-tracker", "transaction-status", "turbine", "udp-client", @@ -378,6 +379,7 @@ solana-test-validator = { path = "test-validator", version = "=2.0.0" } solana-thin-client = { path = "thin-client", version = "=2.0.0" } solana-tpu-client = { path = "tpu-client", version = "=2.0.0", default-features = false } solana-transaction-status = { path = "transaction-status", version = "=2.0.0" } +solana-transaction-metrics-tracker = { path = "transaction-metrics-tracker", version = "=2.0.0" } solana-turbine = { path = "turbine", version = "=2.0.0" } solana-udp-client = { path = "udp-client", version = "=2.0.0" } solana-version = { path = "version", version = "=2.0.0" } diff --git a/core/Cargo.toml b/core/Cargo.toml index e2a936cdabc4c1..1fd25ec38a8d3b 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -67,6 +67,7 @@ solana-send-transaction-service = { workspace = true } solana-streamer = { workspace = true } solana-svm = { workspace = true } solana-tpu-client = { workspace = true } +solana-transaction-metrics-tracker = { workspace = true } solana-transaction-status = { workspace = true } solana-turbine = { workspace = true } solana-unified-scheduler-pool = { workspace = true } diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index c5ed22a34278ce..f417363c7e4409 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -209,6 +209,33 @@ impl Consumer { .slot_metrics_tracker .increment_retryable_packets_count(retryable_transaction_indexes.len() as u64); + // Now we track the performance for the interested transactions which is not in the retryable_transaction_indexes + // We assume the retryable_transaction_indexes is already sorted. + let mut retryable_idx = 0; + for (index, packet) in packets_to_process.iter().enumerate() { + if packet.original_packet().meta().is_perf_track_packet() { + if let Some(start_time) = packet.start_time() { + if retryable_idx >= retryable_transaction_indexes.len() + || retryable_transaction_indexes[retryable_idx] != index + { + let duration = Instant::now().duration_since(*start_time); + + debug!( + "Banking stage processing took {duration:?} for transaction {:?}", + packet.transaction().get_signatures().first() + ); + inc_new_counter_info!( + "txn-metrics-banking-stage-process-us", + duration.as_micros() as usize + ); + } else { + // This packet is retried, advance the retry index to the next, as the next packet's index will + // certainly be > than this. + retryable_idx += 1; + } + } + } + } Some(retryable_transaction_indexes) } diff --git a/core/src/banking_stage/immutable_deserialized_packet.rs b/core/src/banking_stage/immutable_deserialized_packet.rs index 26ede7045d3480..6eb5d68ecaaca5 100644 --- a/core/src/banking_stage/immutable_deserialized_packet.rs +++ b/core/src/banking_stage/immutable_deserialized_packet.rs @@ -13,7 +13,7 @@ use { VersionedTransaction, }, }, - std::{cmp::Ordering, mem::size_of, sync::Arc}, + std::{cmp::Ordering, mem::size_of, sync::Arc, time::Instant}, thiserror::Error, }; @@ -41,10 +41,16 @@ pub struct ImmutableDeserializedPacket { message_hash: Hash, is_simple_vote: bool, compute_budget_details: ComputeBudgetDetails, + banking_stage_start_time: Option, } impl ImmutableDeserializedPacket { pub fn new(packet: Packet) -> Result { + let banking_stage_start_time = packet + .meta() + .is_perf_track_packet() + .then_some(Instant::now()); + let versioned_transaction: VersionedTransaction = packet.deserialize_slice(..)?; let sanitized_transaction = SanitizedVersionedTransaction::try_from(versioned_transaction)?; let message_bytes = packet_message(&packet)?; @@ -67,6 +73,7 @@ impl ImmutableDeserializedPacket { message_hash, is_simple_vote, compute_budget_details, + banking_stage_start_time, }) } @@ -98,6 +105,10 @@ impl ImmutableDeserializedPacket { self.compute_budget_details.clone() } + pub fn start_time(&self) -> &Option { + &self.banking_stage_start_time + } + // This function deserializes packets into transactions, computes the blake3 hash of transaction // messages, and verifies secp256k1 instructions. pub fn build_sanitized_transaction( diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index fcc68050b72d4c..52706f8c2bf63b 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -924,6 +924,7 @@ impl ThreadLocalUnprocessedPackets { .iter() .map(|p| (*p).clone()) .collect_vec(); + let retryable_packets = if let Some(retryable_transaction_indexes) = processing_function(&packets_to_process, payload) { diff --git a/core/src/sigverify_stage.rs b/core/src/sigverify_stage.rs index cde1735611c0d0..18bb9df6d5b96b 100644 --- a/core/src/sigverify_stage.rs +++ b/core/src/sigverify_stage.rs @@ -18,9 +18,11 @@ use { count_discarded_packets, count_packets_in_batches, count_valid_packets, shrink_batches, }, }, - solana_sdk::timing, + solana_sdk::{signature::Signature, timing}, solana_streamer::streamer::{self, StreamerError}, + solana_transaction_metrics_tracker::get_signature_from_packet, std::{ + collections::HashMap, thread::{self, Builder, JoinHandle}, time::Instant, }, @@ -298,8 +300,20 @@ impl SigVerifyStage { verifier: &mut T, stats: &mut SigVerifierStats, ) -> Result<(), T::SendType> { - let (mut batches, num_packets, recv_duration) = streamer::recv_packet_batches(recvr)?; + let mut packet_perf_measure: HashMap<[u8; 64], std::time::Instant> = HashMap::default(); + let (mut batches, num_packets, recv_duration) = streamer::recv_packet_batches(recvr)?; + // track sigverify start time for interested packets + for batch in &batches { + for packet in batch.iter() { + if packet.meta().is_perf_track_packet() { + let signature = get_signature_from_packet(packet); + if let Ok(signature) = signature { + packet_perf_measure.insert(*signature, Instant::now()); + } + } + } + } let batches_len = batches.len(); debug!( "@{:?} verifier: verifying: {}", @@ -372,6 +386,17 @@ impl SigVerifyStage { (num_packets as f32 / verify_time.as_s()) ); + for (signature, start_time) in packet_perf_measure.drain() { + let duration = Instant::now().duration_since(start_time); + debug!( + "Sigverify took {duration:?} for transaction {:?}", + Signature::from(signature) + ); + inc_new_counter_info!( + "txn-metrics-sigverify-packet-verify-us", + duration.as_micros() as usize + ); + } stats .recv_batches_us_hist .increment(recv_duration.as_micros() as u64) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 1043b74c67c619..c11c46af5e3caf 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -4982,6 +4982,7 @@ dependencies = [ "solana-streamer", "solana-svm", "solana-tpu-client", + "solana-transaction-metrics-tracker", "solana-transaction-status", "solana-turbine", "solana-unified-scheduler-pool", @@ -6331,6 +6332,7 @@ dependencies = [ "solana-metrics", "solana-perf", "solana-sdk", + "solana-transaction-metrics-tracker", "thiserror", "tokio", "x509-parser", @@ -6432,6 +6434,20 @@ dependencies = [ "tokio", ] +[[package]] +name = "solana-transaction-metrics-tracker" +version = "1.19.0" +dependencies = [ + "Inflector", + "base64 0.21.7", + "bincode", + "lazy_static", + "log", + "rand 0.8.5", + "solana-perf", + "solana-sdk", +] + [[package]] name = "solana-transaction-status" version = "2.0.0" diff --git a/sdk/src/packet.rs b/sdk/src/packet.rs index faea9ab4753c67..8300b57218c696 100644 --- a/sdk/src/packet.rs +++ b/sdk/src/packet.rs @@ -33,6 +33,8 @@ bitflags! { /// the packet is built. /// This field can be removed when the above feature gate is adopted by mainnet-beta. const ROUND_COMPUTE_UNIT_PRICE = 0b0010_0000; + /// For tracking performance + const PERF_TRACK_PACKET = 0b0100_0000; } } @@ -228,6 +230,12 @@ impl Meta { self.flags.set(PacketFlags::TRACER_PACKET, is_tracer); } + #[inline] + pub fn set_track_performance(&mut self, is_performance_track: bool) { + self.flags + .set(PacketFlags::PERF_TRACK_PACKET, is_performance_track); + } + #[inline] pub fn set_simple_vote(&mut self, is_simple_vote: bool) { self.flags.set(PacketFlags::SIMPLE_VOTE_TX, is_simple_vote); @@ -261,6 +269,11 @@ impl Meta { self.flags.contains(PacketFlags::TRACER_PACKET) } + #[inline] + pub fn is_perf_track_packet(&self) -> bool { + self.flags.contains(PacketFlags::PERF_TRACK_PACKET) + } + #[inline] pub fn round_compute_unit_price(&self) -> bool { self.flags.contains(PacketFlags::ROUND_COMPUTE_UNIT_PRICE) diff --git a/sdk/src/transaction/versioned/sanitized.rs b/sdk/src/transaction/versioned/sanitized.rs index 61ecdfea56bb2a..b6311d5886b0e3 100644 --- a/sdk/src/transaction/versioned/sanitized.rs +++ b/sdk/src/transaction/versioned/sanitized.rs @@ -33,6 +33,10 @@ impl SanitizedVersionedTransaction { &self.message } + pub fn get_signatures(&self) -> &Vec { + &self.signatures + } + /// Consumes the SanitizedVersionedTransaction, returning the fields individually. pub fn destruct(self) -> (Vec, SanitizedVersionedMessage) { (self.signatures, self.message) diff --git a/streamer/Cargo.toml b/streamer/Cargo.toml index 8e1eb12dff1d42..22170e6426c433 100644 --- a/streamer/Cargo.toml +++ b/streamer/Cargo.toml @@ -29,6 +29,7 @@ rustls = { workspace = true, features = ["dangerous_configuration"] } solana-metrics = { workspace = true } solana-perf = { workspace = true } solana-sdk = { workspace = true } +solana-transaction-metrics-tracker = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["full"] } x509-parser = { workspace = true } diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index 225412dd08b315..23234231fd2750 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -27,10 +27,14 @@ use { QUIC_MIN_STAKED_CONCURRENT_STREAMS, QUIC_MIN_STAKED_RECEIVE_WINDOW_RATIO, QUIC_TOTAL_STAKED_CONCURRENT_STREAMS, QUIC_UNSTAKED_RECEIVE_WINDOW_RATIO, }, - signature::Keypair, + signature::{Keypair, Signature}, timing, }, + solana_transaction_metrics_tracker::{ + get_signature_from_packet, signature_if_should_track_packet, + }, std::{ + collections::HashMap, iter::repeat_with, net::{IpAddr, SocketAddr, UdpSocket}, sync::{ @@ -81,6 +85,7 @@ struct PacketChunk { struct PacketAccumulator { pub meta: Meta, pub chunks: Vec, + pub start_time: Instant, } #[derive(Copy, Clone, Debug)] @@ -628,6 +633,7 @@ async fn packet_batch_sender( trace!("enter packet_batch_sender"); let mut batch_start_time = Instant::now(); loop { + let mut packet_perf_measure: HashMap<[u8; 64], std::time::Instant> = HashMap::default(); let mut packet_batch = PacketBatch::with_capacity(PACKETS_PER_BATCH); let mut total_bytes: usize = 0; @@ -647,6 +653,7 @@ async fn packet_batch_sender( || (!packet_batch.is_empty() && elapsed >= coalesce) { let len = packet_batch.len(); + track_streamer_fetch_packet_performance(&packet_batch, &mut packet_perf_measure); if let Err(e) = packet_sender.send(packet_batch) { stats .total_packet_batch_send_err @@ -692,6 +699,13 @@ async fn packet_batch_sender( total_bytes += packet_batch[i].meta().size; + if let Some(signature) = + signature_if_should_track_packet(&packet_batch[i]).unwrap_or(None) + { + packet_perf_measure.insert(*signature, packet_accumulator.start_time); + // we set the PERF_TRACK_PACKET on + packet_batch[i].meta_mut().set_track_performance(true); + } stats .total_chunks_processed_by_batcher .fetch_add(num_chunks, Ordering::Relaxed); @@ -700,6 +714,30 @@ async fn packet_batch_sender( } } +fn track_streamer_fetch_packet_performance( + packet_batch: &PacketBatch, + packet_perf_measure: &mut HashMap<[u8; 64], Instant>, +) { + for packet in packet_batch.iter() { + if packet.meta().is_perf_track_packet() { + let signature = get_signature_from_packet(packet); + if let Ok(signature) = signature { + if let Some(start_time) = packet_perf_measure.remove(signature) { + let duration = Instant::now().duration_since(start_time); + debug!( + "QUIC streamer fetch stage took {duration:?} for transaction {:?}", + Signature::from(*signature) + ); + inc_new_counter_info!( + "txn-metrics-quic-streamer-packet-fetch-us", + duration.as_micros() as usize + ); + } + } + } + } +} + async fn handle_connection( connection: Connection, remote_addr: SocketAddr, @@ -854,6 +892,7 @@ async fn handle_chunk( *packet_accum = Some(PacketAccumulator { meta, chunks: Vec::new(), + start_time: Instant::now(), }); } @@ -1453,6 +1492,7 @@ pub mod test { offset, end_of_chunk: size, }], + start_time: Instant::now(), }; ptk_sender.send(packet_accum).await.unwrap(); } diff --git a/transaction-metrics-tracker/Cargo.toml b/transaction-metrics-tracker/Cargo.toml new file mode 100644 index 00000000000000..9bd82702a3ebb4 --- /dev/null +++ b/transaction-metrics-tracker/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "solana-transaction-metrics-tracker" +description = "Solana transaction metrics tracker" +documentation = "https://docs.rs/solana-transaction-metrics-tracker" +version = { workspace = true } +authors = { workspace = true } +repository = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +edition = { workspace = true } +publish = false + +[dependencies] +Inflector = { workspace = true } +base64 = { workspace = true } +bincode = { workspace = true } +# Update this borsh dependency to the workspace version once +lazy_static = { workspace = true } +log = { workspace = true } +rand = { workspace = true } +solana-perf = { workspace = true } +solana-sdk = { workspace = true } + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] diff --git a/transaction-metrics-tracker/src/lib.rs b/transaction-metrics-tracker/src/lib.rs new file mode 100644 index 00000000000000..02ae2c14fa8ca5 --- /dev/null +++ b/transaction-metrics-tracker/src/lib.rs @@ -0,0 +1,54 @@ +use { + lazy_static::lazy_static, + log::*, + rand::Rng, + solana_perf::sigverify::PacketError, + solana_sdk::{packet::Packet, short_vec::decode_shortu16_len, signature::SIGNATURE_BYTES}, +}; + +// The mask is 12 bits long (1<<12 = 4096), it means the probability of matching +// the transaction is 1/4096 assuming the portion being matched is random. +lazy_static! { + static ref TXN_MASK: u16 = rand::thread_rng().gen_range(0..4096); +} + +/// Check if a transaction given its signature matches the randomly selected mask. +/// The signaure should be from the reference of Signature +pub fn should_track_transaction(signature: &[u8; SIGNATURE_BYTES]) -> bool { + // We do not use the highest signature byte as it is not really random + let match_portion: u16 = u16::from_le_bytes([signature[61], signature[62]]) >> 4; + trace!("Matching txn: {match_portion:b} {:b}", *TXN_MASK); + *TXN_MASK == match_portion +} + +/// Check if a transaction packet's signature matches the mask. +/// This does a rudimentry verification to make sure the packet at least +/// contains the signature data and it returns the reference to the signature. +pub fn signature_if_should_track_packet( + packet: &Packet, +) -> Result, PacketError> { + let signature = get_signature_from_packet(packet)?; + Ok(should_track_transaction(signature).then_some(signature)) +} + +/// Get the signature of the transaction packet +/// This does a rudimentry verification to make sure the packet at least +/// contains the signature data and it returns the reference to the signature. +pub fn get_signature_from_packet(packet: &Packet) -> Result<&[u8; SIGNATURE_BYTES], PacketError> { + let (sig_len_untrusted, sig_start) = packet + .data(..) + .and_then(|bytes| decode_shortu16_len(bytes).ok()) + .ok_or(PacketError::InvalidShortVec)?; + + if sig_len_untrusted < 1 { + return Err(PacketError::InvalidSignatureLen); + } + + let signature = packet + .data(sig_start..sig_start.saturating_add(SIGNATURE_BYTES)) + .ok_or(PacketError::InvalidSignatureLen)?; + let signature = signature + .try_into() + .map_err(|_| PacketError::InvalidSignatureLen)?; + Ok(signature) +} From 6ac031bd414aa62a2a476aadfa6d206c5f4a09d8 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 03:03:57 -0800 Subject: [PATCH 02/42] Use datapoint as opposed to counters --- core/src/banking_stage/consumer.rs | 7 ++-- core/src/banking_stage/leader_slot_metrics.rs | 11 ++++++ .../leader_slot_timing_metrics.rs | 25 +++++++++++++ core/src/sigverify_stage.rs | 35 +++++++++++++++---- streamer/src/nonblocking/quic.rs | 23 ++++++++---- streamer/src/quic.rs | 26 +++++++++++++- 6 files changed, 109 insertions(+), 18 deletions(-) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index f417363c7e4409..9e9475f7678e3c 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -224,10 +224,9 @@ impl Consumer { "Banking stage processing took {duration:?} for transaction {:?}", packet.transaction().get_signatures().first() ); - inc_new_counter_info!( - "txn-metrics-banking-stage-process-us", - duration.as_micros() as usize - ); + payload + .slot_metrics_tracker + .increment_process_sampled_packets_us(duration.as_micros() as u64); } else { // This packet is retried, advance the retry index to the next, as the next packet's index will // certainly be > than this. diff --git a/core/src/banking_stage/leader_slot_metrics.rs b/core/src/banking_stage/leader_slot_metrics.rs index 88ea6b5ee340cf..1c255ca019bfe7 100644 --- a/core/src/banking_stage/leader_slot_metrics.rs +++ b/core/src/banking_stage/leader_slot_metrics.rs @@ -936,6 +936,17 @@ impl LeaderSlotMetricsTracker { ); } } + + pub(crate) fn increment_process_sampled_packets_us(&mut self, us: u64) { + if let Some(leader_slot_metrics) = &mut self.leader_slot_metrics { + leader_slot_metrics + .timing_metrics + .process_packets_timings + .process_sampled_packets_us_hist + .increment(us) + .unwrap(); + } + } } #[cfg(test)] diff --git a/core/src/banking_stage/leader_slot_timing_metrics.rs b/core/src/banking_stage/leader_slot_timing_metrics.rs index 7727b6cf6c6563..34ce64b31c34f3 100644 --- a/core/src/banking_stage/leader_slot_timing_metrics.rs +++ b/core/src/banking_stage/leader_slot_timing_metrics.rs @@ -244,6 +244,9 @@ pub(crate) struct ProcessPacketsTimings { // Time spent running the cost model in processing transactions before executing // transactions pub cost_model_us: u64, + + // banking stage processing time histogram for sampled packets + pub process_sampled_packets_us_hist: histogram::Histogram, } impl ProcessPacketsTimings { @@ -264,6 +267,28 @@ impl ProcessPacketsTimings { i64 ), ("cost_model_us", self.cost_model_us, i64), + ( + "process_sampled_packets_us_90pct", + self.process_sampled_packets_us_hist + .percentile(90.0) + .unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_min", + self.process_sampled_packets_us_hist.minimum().unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_max", + self.process_sampled_packets_us_hist.maximum().unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_mean", + self.process_sampled_packets_us_hist.mean().unwrap_or(0), + i64 + ), ); } } diff --git a/core/src/sigverify_stage.rs b/core/src/sigverify_stage.rs index 18bb9df6d5b96b..5b9112f87a1622 100644 --- a/core/src/sigverify_stage.rs +++ b/core/src/sigverify_stage.rs @@ -80,8 +80,9 @@ struct SigVerifierStats { verify_batches_pp_us_hist: histogram::Histogram, // per-packet time to call verify_batch discard_packets_pp_us_hist: histogram::Histogram, // per-packet time to call verify_batch dedup_packets_pp_us_hist: histogram::Histogram, // per-packet time to call verify_batch - batches_hist: histogram::Histogram, // number of packet batches per verify call - packets_hist: histogram::Histogram, // number of packets per verify call + sampled_packets_pp_us_hist: histogram::Histogram, // per-packet time do do overall verify for sampled packets + batches_hist: histogram::Histogram, // number of packet batches per verify call + packets_hist: histogram::Histogram, // number of packets per verify call num_deduper_saturations: usize, total_batches: usize, total_packets: usize, @@ -183,6 +184,28 @@ impl SigVerifierStats { self.dedup_packets_pp_us_hist.mean().unwrap_or(0), i64 ), + ( + "sampled_verify_packets_pp_us_90pct", + self.sampled_packets_pp_us_hist + .percentile(90.0) + .unwrap_or(0), + i64 + ), + ( + "sampled_verify_packets_pp_us_min", + self.sampled_packets_pp_us_hist.minimum().unwrap_or(0), + i64 + ), + ( + "sampled_verify_packets_pp_us_max", + self.sampled_packets_pp_us_hist.maximum().unwrap_or(0), + i64 + ), + ( + "sampled_verify_packets_pp_us_mean", + self.sampled_packets_pp_us_hist.mean().unwrap_or(0), + i64 + ), ( "batches_90pct", self.batches_hist.percentile(90.0).unwrap_or(0), @@ -392,10 +415,10 @@ impl SigVerifyStage { "Sigverify took {duration:?} for transaction {:?}", Signature::from(signature) ); - inc_new_counter_info!( - "txn-metrics-sigverify-packet-verify-us", - duration.as_micros() as usize - ); + stats + .sampled_packets_pp_us_hist + .increment(duration.as_micros() as u64) + .unwrap(); } stats .recv_batches_us_hist diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index 23234231fd2750..b3d02d8287b77e 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -653,7 +653,12 @@ async fn packet_batch_sender( || (!packet_batch.is_empty() && elapsed >= coalesce) { let len = packet_batch.len(); - track_streamer_fetch_packet_performance(&packet_batch, &mut packet_perf_measure); + track_streamer_fetch_packet_performance( + &packet_batch, + &mut packet_perf_measure, + stats.clone(), + ); + if let Err(e) = packet_sender.send(packet_batch) { stats .total_packet_batch_send_err @@ -699,8 +704,9 @@ async fn packet_batch_sender( total_bytes += packet_batch[i].meta().size; - if let Some(signature) = - signature_if_should_track_packet(&packet_batch[i]).unwrap_or(None) + if let Some(signature) = signature_if_should_track_packet(&packet_batch[i]) + .ok() + .flatten() { packet_perf_measure.insert(*signature, packet_accumulator.start_time); // we set the PERF_TRACK_PACKET on @@ -717,6 +723,7 @@ async fn packet_batch_sender( fn track_streamer_fetch_packet_performance( packet_batch: &PacketBatch, packet_perf_measure: &mut HashMap<[u8; 64], Instant>, + stats: Arc, ) { for packet in packet_batch.iter() { if packet.meta().is_perf_track_packet() { @@ -728,10 +735,12 @@ fn track_streamer_fetch_packet_performance( "QUIC streamer fetch stage took {duration:?} for transaction {:?}", Signature::from(*signature) ); - inc_new_counter_info!( - "txn-metrics-quic-streamer-packet-fetch-us", - duration.as_micros() as usize - ); + stats + .process_sampled_packets_us_hist + .lock() + .unwrap() + .increment(duration.as_micros() as u64) + .unwrap(); } } } diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index a7a08c73f4833b..152572d1fe218a 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -17,7 +17,7 @@ use { net::UdpSocket, sync::{ atomic::{AtomicBool, AtomicUsize, Ordering}, - Arc, RwLock, + Arc, Mutex, RwLock, }, thread, time::{Duration, SystemTime}, @@ -175,10 +175,12 @@ pub struct StreamStats { pub(crate) stream_load_ema: AtomicUsize, pub(crate) stream_load_ema_overflow: AtomicUsize, pub(crate) stream_load_capacity_overflow: AtomicUsize, + pub(crate) process_sampled_packets_us_hist: Mutex, } impl StreamStats { pub fn report(&self, name: &'static str) { + let process_sampled_packets_us_hist = self.process_sampled_packets_us_hist.lock().unwrap(); datapoint_info!( name, ( @@ -425,6 +427,28 @@ impl StreamStats { self.stream_load_capacity_overflow.load(Ordering::Relaxed), i64 ), + ( + "process_sampled_packets_us_90pct", + process_sampled_packets_us_hist + .percentile(90.0) + .unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_min", + process_sampled_packets_us_hist.minimum().unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_max", + process_sampled_packets_us_hist.maximum().unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_mean", + process_sampled_packets_us_hist.mean().unwrap_or(0), + i64 + ), ); } } From 3d291e3b919b630b940a5a4319c1035d324800b1 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 04:07:20 -0800 Subject: [PATCH 03/42] Add a unit test --- transaction-metrics-tracker/src/lib.rs | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/transaction-metrics-tracker/src/lib.rs b/transaction-metrics-tracker/src/lib.rs index 02ae2c14fa8ca5..4b770eddb6c59b 100644 --- a/transaction-metrics-tracker/src/lib.rs +++ b/transaction-metrics-tracker/src/lib.rs @@ -40,6 +40,7 @@ pub fn get_signature_from_packet(packet: &Packet) -> Result<&[u8; SIGNATURE_BYTE .and_then(|bytes| decode_shortu16_len(bytes).ok()) .ok_or(PacketError::InvalidShortVec)?; + println!("Sig length is okay!!"); if sig_len_untrusted < 1 { return Err(PacketError::InvalidSignatureLen); } @@ -52,3 +53,31 @@ pub fn get_signature_from_packet(packet: &Packet) -> Result<&[u8; SIGNATURE_BYTE .map_err(|_| PacketError::InvalidSignatureLen)?; Ok(signature) } + +#[cfg(test)] +mod tests { + use { + super::*, + solana_sdk::{hash::Hash, signature::Keypair, system_transaction}, + }; + + #[test] + fn test_get_signature_from_packet() { + // default invalid txn packet + let packet = Packet::default(); + let sig = get_signature_from_packet(&packet); + assert_eq!(sig, Err(PacketError::InvalidShortVec)); + + // Use a valid transaction, it should succeed + let tx = system_transaction::transfer( + &Keypair::new(), + &solana_sdk::pubkey::new_rand(), + 1, + Hash::new_unique(), + ); + let packet = Packet::from_data(None, tx).unwrap(); + + let sig = get_signature_from_packet(&packet); + assert!(sig.is_ok()); + } +} From d84f335cf8bccf521a97226a88dc6add24c845a7 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 04:10:07 -0800 Subject: [PATCH 04/42] removed a print --- transaction-metrics-tracker/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/transaction-metrics-tracker/src/lib.rs b/transaction-metrics-tracker/src/lib.rs index 4b770eddb6c59b..02d7c03d94f068 100644 --- a/transaction-metrics-tracker/src/lib.rs +++ b/transaction-metrics-tracker/src/lib.rs @@ -40,7 +40,6 @@ pub fn get_signature_from_packet(packet: &Packet) -> Result<&[u8; SIGNATURE_BYTE .and_then(|bytes| decode_shortu16_len(bytes).ok()) .ok_or(PacketError::InvalidShortVec)?; - println!("Sig length is okay!!"); if sig_len_untrusted < 1 { return Err(PacketError::InvalidSignatureLen); } From 07a06ca928e0c6e97f4f3dc592cb7dc40ae78948 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 12:29:26 -0800 Subject: [PATCH 05/42] Added more unit tests --- transaction-metrics-tracker/src/lib.rs | 83 ++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 4 deletions(-) diff --git a/transaction-metrics-tracker/src/lib.rs b/transaction-metrics-tracker/src/lib.rs index 02d7c03d94f068..4ffe7fda9d7ece 100644 --- a/transaction-metrics-tracker/src/lib.rs +++ b/transaction-metrics-tracker/src/lib.rs @@ -17,7 +17,7 @@ lazy_static! { pub fn should_track_transaction(signature: &[u8; SIGNATURE_BYTES]) -> bool { // We do not use the highest signature byte as it is not really random let match_portion: u16 = u16::from_le_bytes([signature[61], signature[62]]) >> 4; - trace!("Matching txn: {match_portion:b} {:b}", *TXN_MASK); + trace!("Matching txn: {match_portion:016b} {:016b}", *TXN_MASK); *TXN_MASK == match_portion } @@ -57,12 +57,16 @@ pub fn get_signature_from_packet(packet: &Packet) -> Result<&[u8; SIGNATURE_BYTE mod tests { use { super::*, - solana_sdk::{hash::Hash, signature::Keypair, system_transaction}, + solana_sdk::{ + hash::Hash, + signature::{Keypair, Signature}, + system_transaction, + }, }; #[test] fn test_get_signature_from_packet() { - // default invalid txn packet + // Default invalid txn packet let packet = Packet::default(); let sig = get_signature_from_packet(&packet); assert_eq!(sig, Err(PacketError::InvalidShortVec)); @@ -74,9 +78,80 @@ mod tests { 1, Hash::new_unique(), ); - let packet = Packet::from_data(None, tx).unwrap(); + let mut packet = Packet::from_data(None, tx).unwrap(); let sig = get_signature_from_packet(&packet); assert!(sig.is_ok()); + + // Invalid signature length + packet.buffer_mut()[0] = 0x0; + let sig = get_signature_from_packet(&packet); + assert_eq!(sig, Err(PacketError::InvalidSignatureLen)); + } + + #[test] + fn test_should_track_transaction() { + let mut sig = [0x0; SIGNATURE_BYTES]; + let track = should_track_transaction(&sig); + assert!(!track); + + // Intentionally matching the randomly generated mask + // The lower four bits are ignored as only 12 highest bits from + // signature's 61 and 62 u8 are used for matching. + // We generate a random one + let mut rng = rand::thread_rng(); + let random_number: u8 = rng.gen_range(0..=15); + sig[61] = ((*TXN_MASK & 0xf as u16) << 4) as u8 | random_number; + sig[62] = (*TXN_MASK >> 4) as u8; + + let track = should_track_transaction(&sig); + assert!(track); + } + + #[test] + fn test_signature_if_should_track_packet() { + // Default invalid txn packet + let packet = Packet::default(); + let sig = signature_if_should_track_packet(&packet); + assert_eq!(sig, Err(PacketError::InvalidShortVec)); + + // Use a valid transaction which is not matched + let tx = system_transaction::transfer( + &Keypair::new(), + &solana_sdk::pubkey::new_rand(), + 1, + Hash::new_unique(), + ); + let packet = Packet::from_data(None, tx).unwrap(); + let sig = signature_if_should_track_packet(&packet); + assert_eq!(Ok(None), sig); + + // Now simulate a txn matching the signature mask + let mut tx = system_transaction::transfer( + &Keypair::new(), + &solana_sdk::pubkey::new_rand(), + 1, + Hash::new_unique(), + ); + let mut sig = [0x0; SIGNATURE_BYTES]; + sig[61] = ((*TXN_MASK & 0xf as u16) << 4) as u8; + sig[62] = (*TXN_MASK >> 4) as u8; + + let sig = Signature::from(sig); + tx.signatures[0] = sig; + let mut packet = Packet::from_data(None, tx).unwrap(); + let sig2 = signature_if_should_track_packet(&packet); + + match sig2 { + Ok(sig) => { + assert!(sig.is_some()); + } + Err(_) => assert!(false, "Expected to get a matching signature!"), + } + + // Invalid signature length + packet.buffer_mut()[0] = 0x0; + let sig = signature_if_should_track_packet(&packet); + assert_eq!(sig, Err(PacketError::InvalidSignatureLen)); } } From 529bcc83d93ab3def217d94e16cc96c8fc5c7afc Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:08:46 -0800 Subject: [PATCH 06/42] Making stats names consistent across layers --- core/src/sigverify_stage.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/core/src/sigverify_stage.rs b/core/src/sigverify_stage.rs index 5b9112f87a1622..7cce095695f445 100644 --- a/core/src/sigverify_stage.rs +++ b/core/src/sigverify_stage.rs @@ -80,9 +80,9 @@ struct SigVerifierStats { verify_batches_pp_us_hist: histogram::Histogram, // per-packet time to call verify_batch discard_packets_pp_us_hist: histogram::Histogram, // per-packet time to call verify_batch dedup_packets_pp_us_hist: histogram::Histogram, // per-packet time to call verify_batch - sampled_packets_pp_us_hist: histogram::Histogram, // per-packet time do do overall verify for sampled packets - batches_hist: histogram::Histogram, // number of packet batches per verify call - packets_hist: histogram::Histogram, // number of packets per verify call + process_sampled_packets_us_hist: histogram::Histogram, // per-packet time do do overall verify for sampled packets + batches_hist: histogram::Histogram, // number of packet batches per verify call + packets_hist: histogram::Histogram, // number of packets per verify call num_deduper_saturations: usize, total_batches: usize, total_packets: usize, @@ -185,25 +185,25 @@ impl SigVerifierStats { i64 ), ( - "sampled_verify_packets_pp_us_90pct", - self.sampled_packets_pp_us_hist + "process_sampled_packets_us_90pct", + self.process_sampled_packets_us_hist .percentile(90.0) .unwrap_or(0), i64 ), ( - "sampled_verify_packets_pp_us_min", - self.sampled_packets_pp_us_hist.minimum().unwrap_or(0), + "process_sampled_packets_us_min", + self.process_sampled_packets_us_hist.minimum().unwrap_or(0), i64 ), ( - "sampled_verify_packets_pp_us_max", - self.sampled_packets_pp_us_hist.maximum().unwrap_or(0), + "process_sampled_packets_us_max", + self.process_sampled_packets_us_hist.maximum().unwrap_or(0), i64 ), ( - "sampled_verify_packets_pp_us_mean", - self.sampled_packets_pp_us_hist.mean().unwrap_or(0), + "process_sampled_packets_us_mean", + self.process_sampled_packets_us_hist.mean().unwrap_or(0), i64 ), ( @@ -416,7 +416,7 @@ impl SigVerifyStage { Signature::from(signature) ); stats - .sampled_packets_pp_us_hist + .process_sampled_packets_us_hist .increment(duration.as_micros() as u64) .unwrap(); } From a76f589ef4403c320652443e9529d3d54cd7d8eb Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 14:17:44 -0800 Subject: [PATCH 07/42] Added more unit tests --- transaction-metrics-tracker/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/transaction-metrics-tracker/src/lib.rs b/transaction-metrics-tracker/src/lib.rs index 4ffe7fda9d7ece..36c0349620275e 100644 --- a/transaction-metrics-tracker/src/lib.rs +++ b/transaction-metrics-tracker/src/lib.rs @@ -134,7 +134,7 @@ mod tests { Hash::new_unique(), ); let mut sig = [0x0; SIGNATURE_BYTES]; - sig[61] = ((*TXN_MASK & 0xf as u16) << 4) as u8; + sig[61] = ((*TXN_MASK & 0xf_u16) << 4) as u8; sig[62] = (*TXN_MASK >> 4) as u8; let sig = Signature::from(sig); @@ -146,7 +146,7 @@ mod tests { Ok(sig) => { assert!(sig.is_some()); } - Err(_) => assert!(false, "Expected to get a matching signature!"), + Err(_) => panic!("Expected to get a matching signature!"), } // Invalid signature length From 1ac2493037532c75f654250e72893b22d7da304f Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 14:55:09 -0800 Subject: [PATCH 08/42] Added more unit tests --- transaction-metrics-tracker/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction-metrics-tracker/src/lib.rs b/transaction-metrics-tracker/src/lib.rs index 36c0349620275e..2baec195de9b84 100644 --- a/transaction-metrics-tracker/src/lib.rs +++ b/transaction-metrics-tracker/src/lib.rs @@ -101,7 +101,7 @@ mod tests { // We generate a random one let mut rng = rand::thread_rng(); let random_number: u8 = rng.gen_range(0..=15); - sig[61] = ((*TXN_MASK & 0xf as u16) << 4) as u8 | random_number; + sig[61] = ((*TXN_MASK & 0xf_u16) << 4) as u8 | random_number; sig[62] = (*TXN_MASK >> 4) as u8; let track = should_track_transaction(&sig); From 67174d05e63089d2d4529f457165a86a70385ffe Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 21:31:14 -0800 Subject: [PATCH 09/42] Do not use binary_search, do simple compare in one loop --- streamer/src/nonblocking/quic.rs | 53 ++++++++++++-------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index b3d02d8287b77e..901076da10f9d0 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -30,11 +30,8 @@ use { signature::{Keypair, Signature}, timing, }, - solana_transaction_metrics_tracker::{ - get_signature_from_packet, signature_if_should_track_packet, - }, + solana_transaction_metrics_tracker::signature_if_should_track_packet, std::{ - collections::HashMap, iter::repeat_with, net::{IpAddr, SocketAddr, UdpSocket}, sync::{ @@ -633,7 +630,7 @@ async fn packet_batch_sender( trace!("enter packet_batch_sender"); let mut batch_start_time = Instant::now(); loop { - let mut packet_perf_measure: HashMap<[u8; 64], std::time::Instant> = HashMap::default(); + let mut packet_perf_measure: Vec<([u8; 64], std::time::Instant)> = Vec::default(); let mut packet_batch = PacketBatch::with_capacity(PACKETS_PER_BATCH); let mut total_bytes: usize = 0; @@ -653,11 +650,7 @@ async fn packet_batch_sender( || (!packet_batch.is_empty() && elapsed >= coalesce) { let len = packet_batch.len(); - track_streamer_fetch_packet_performance( - &packet_batch, - &mut packet_perf_measure, - stats.clone(), - ); + track_streamer_fetch_packet_performance(&mut packet_perf_measure, &stats); if let Err(e) = packet_sender.send(packet_batch) { stats @@ -708,7 +701,7 @@ async fn packet_batch_sender( .ok() .flatten() { - packet_perf_measure.insert(*signature, packet_accumulator.start_time); + packet_perf_measure.push((*signature, packet_accumulator.start_time)); // we set the PERF_TRACK_PACKET on packet_batch[i].meta_mut().set_track_performance(true); } @@ -721,29 +714,23 @@ async fn packet_batch_sender( } fn track_streamer_fetch_packet_performance( - packet_batch: &PacketBatch, - packet_perf_measure: &mut HashMap<[u8; 64], Instant>, - stats: Arc, + packet_perf_measure: &mut Vec<([u8; 64], Instant)>, + stats: &Arc, ) { - for packet in packet_batch.iter() { - if packet.meta().is_perf_track_packet() { - let signature = get_signature_from_packet(packet); - if let Ok(signature) = signature { - if let Some(start_time) = packet_perf_measure.remove(signature) { - let duration = Instant::now().duration_since(start_time); - debug!( - "QUIC streamer fetch stage took {duration:?} for transaction {:?}", - Signature::from(*signature) - ); - stats - .process_sampled_packets_us_hist - .lock() - .unwrap() - .increment(duration.as_micros() as u64) - .unwrap(); - } - } - } + if packet_perf_measure.is_empty() { + return; + } + let mut process_sampled_packets_us_hist = stats.process_sampled_packets_us_hist.lock().unwrap(); + + for (signature, start_time) in packet_perf_measure.iter() { + let duration = Instant::now().duration_since(*start_time); + debug!( + "QUIC streamer fetch stage took {duration:?} for transaction {:?}", + Signature::from(*signature) + ); + process_sampled_packets_us_hist + .increment(duration.as_micros() as u64) + .unwrap(); } } From dac53e72a21c8f7548478ea36f4d4046bc5c7556 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Mon, 11 Mar 2024 22:31:00 -0700 Subject: [PATCH 10/42] Cargo.lock change --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 604c4ef598c029..c686695ca3696c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7449,7 +7449,7 @@ dependencies = [ [[package]] name = "solana-transaction-metrics-tracker" -version = "1.19.0" +version = "2.0.0" dependencies = [ "Inflector", "base64 0.21.7", From 4cdca1c4d733fd405eb52237001228aec9e915cf Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 21:47:32 -0800 Subject: [PATCH 11/42] measure perf track overhead --- Cargo.lock | 1 + streamer/Cargo.toml | 1 + streamer/src/nonblocking/quic.rs | 6 ++++++ streamer/src/quic.rs | 8 +++++++- 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c686695ca3696c..3459ace3b811a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7277,6 +7277,7 @@ dependencies = [ "rand 0.8.5", "rustls", "solana-logger", + "solana-measure", "solana-metrics", "solana-perf", "solana-sdk", diff --git a/streamer/Cargo.toml b/streamer/Cargo.toml index 22170e6426c433..55d0030e734607 100644 --- a/streamer/Cargo.toml +++ b/streamer/Cargo.toml @@ -26,6 +26,7 @@ quinn = { workspace = true } quinn-proto = { workspace = true } rand = { workspace = true } rustls = { workspace = true, features = ["dangerous_configuration"] } +solana-measure = { workspace = true } solana-metrics = { workspace = true } solana-perf = { workspace = true } solana-sdk = { workspace = true } diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index 901076da10f9d0..0a5b0ef3fe6c03 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -17,6 +17,7 @@ use { quinn::{Connecting, Connection, Endpoint, EndpointConfig, TokioRuntime, VarInt}, quinn_proto::VarIntBoundsExceeded, rand::{thread_rng, Rng}, + solana_measure::measure::Measure, solana_perf::packet::{PacketBatch, PACKETS_PER_BATCH}, solana_sdk::{ packet::{Meta, PACKET_DATA_SIZE}, @@ -720,6 +721,7 @@ fn track_streamer_fetch_packet_performance( if packet_perf_measure.is_empty() { return; } + let mut measure = Measure::start("track_perf"); let mut process_sampled_packets_us_hist = stats.process_sampled_packets_us_hist.lock().unwrap(); for (signature, start_time) in packet_perf_measure.iter() { @@ -732,6 +734,10 @@ fn track_streamer_fetch_packet_performance( .increment(duration.as_micros() as u64) .unwrap(); } + measure.stop(); + stats + .perf_track_overhead_us + .fetch_add(measure.as_us(), Ordering::Relaxed); } async fn handle_connection( diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index 152572d1fe218a..65e1c571a05467 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -16,7 +16,7 @@ use { std::{ net::UdpSocket, sync::{ - atomic::{AtomicBool, AtomicUsize, Ordering}, + atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}, Arc, Mutex, RwLock, }, thread, @@ -176,6 +176,7 @@ pub struct StreamStats { pub(crate) stream_load_ema_overflow: AtomicUsize, pub(crate) stream_load_capacity_overflow: AtomicUsize, pub(crate) process_sampled_packets_us_hist: Mutex, + pub(crate) perf_track_overhead_us: AtomicU64, } impl StreamStats { @@ -449,6 +450,11 @@ impl StreamStats { process_sampled_packets_us_hist.mean().unwrap_or(0), i64 ), + ( + "perf_track_overhead_us", + self.perf_track_overhead_us.swap(0, Ordering::Relaxed), + i64 + ), ); } } From a6a530a7de3fd0ffb4568d1bf603a6fa93cd119a Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Thu, 29 Feb 2024 21:48:45 -0800 Subject: [PATCH 12/42] missing cargo.lock --- programs/sbf/Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index c11c46af5e3caf..51fa9e165efc2e 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -6329,6 +6329,7 @@ dependencies = [ "quinn-proto", "rand 0.8.5", "rustls", + "solana-measure", "solana-metrics", "solana-perf", "solana-sdk", From c14ac9915190b47c3807f1a55dfb6eca2a6f21d0 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Fri, 1 Mar 2024 01:47:03 -0800 Subject: [PATCH 13/42] Do not use Hashmap for perf track. Using vec. Measure the overhead of perf track --- core/src/sigverify_stage.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/core/src/sigverify_stage.rs b/core/src/sigverify_stage.rs index 7cce095695f445..aa9f4b7c4a2869 100644 --- a/core/src/sigverify_stage.rs +++ b/core/src/sigverify_stage.rs @@ -22,7 +22,6 @@ use { solana_streamer::streamer::{self, StreamerError}, solana_transaction_metrics_tracker::get_signature_from_packet, std::{ - collections::HashMap, thread::{self, Builder, JoinHandle}, time::Instant, }, @@ -96,6 +95,7 @@ struct SigVerifierStats { total_discard_random_time_us: usize, total_verify_time_us: usize, total_shrink_time_us: usize, + perf_track_overhead_us: usize, } impl SigVerifierStats { @@ -239,6 +239,7 @@ impl SigVerifierStats { ), ("total_verify_time_us", self.total_verify_time_us, i64), ("total_shrink_time_us", self.total_shrink_time_us, i64), + ("perf_track_overhead_us", self.perf_track_overhead_us, i64), ); } } @@ -323,20 +324,26 @@ impl SigVerifyStage { verifier: &mut T, stats: &mut SigVerifierStats, ) -> Result<(), T::SendType> { - let mut packet_perf_measure: HashMap<[u8; 64], std::time::Instant> = HashMap::default(); + let mut packet_perf_measure: Vec<([u8; 64], std::time::Instant)> = Vec::default(); let (mut batches, num_packets, recv_duration) = streamer::recv_packet_batches(recvr)?; + + let mut start_perf_track_measure = Measure::start("start_perf_track"); // track sigverify start time for interested packets for batch in &batches { for packet in batch.iter() { if packet.meta().is_perf_track_packet() { let signature = get_signature_from_packet(packet); if let Ok(signature) = signature { - packet_perf_measure.insert(*signature, Instant::now()); + packet_perf_measure.push((*signature, Instant::now())); } } } } + start_perf_track_measure.stop(); + + stats.perf_track_overhead_us = start_perf_track_measure.as_us() as usize; + let batches_len = batches.len(); debug!( "@{:?} verifier: verifying: {}", @@ -409,17 +416,22 @@ impl SigVerifyStage { (num_packets as f32 / verify_time.as_s()) ); - for (signature, start_time) in packet_perf_measure.drain() { - let duration = Instant::now().duration_since(start_time); + let mut perf_track_end_measure = Measure::start("perf_track_end"); + for (signature, start_time) in packet_perf_measure.iter() { + let duration = Instant::now().duration_since(*start_time); debug!( "Sigverify took {duration:?} for transaction {:?}", - Signature::from(signature) + Signature::from(*signature) ); stats .process_sampled_packets_us_hist .increment(duration.as_micros() as u64) .unwrap(); } + + perf_track_end_measure.stop(); + stats.perf_track_overhead_us += perf_track_end_measure.as_us() as usize; + stats .recv_batches_us_hist .increment(recv_duration.as_micros() as u64) From d1221621dddd7fb17c5556b02be78c0a822bcd83 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Fri, 1 Mar 2024 02:03:37 -0800 Subject: [PATCH 14/42] Clippy issue --- streamer/src/nonblocking/quic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/streamer/src/nonblocking/quic.rs b/streamer/src/nonblocking/quic.rs index 0a5b0ef3fe6c03..3485e4fe585d06 100644 --- a/streamer/src/nonblocking/quic.rs +++ b/streamer/src/nonblocking/quic.rs @@ -715,7 +715,7 @@ async fn packet_batch_sender( } fn track_streamer_fetch_packet_performance( - packet_perf_measure: &mut Vec<([u8; 64], Instant)>, + packet_perf_measure: &mut [([u8; 64], Instant)], stats: &Arc, ) { if packet_perf_measure.is_empty() { From d94ee5bcc4de65e6acd2388a99a4577a74c59025 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 5 Mar 2024 10:21:04 -0800 Subject: [PATCH 15/42] clear histogram for streamer stage --- streamer/src/quic.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index 65e1c571a05467..a8d4cd01291cd2 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -181,7 +181,8 @@ pub struct StreamStats { impl StreamStats { pub fn report(&self, name: &'static str) { - let process_sampled_packets_us_hist = self.process_sampled_packets_us_hist.lock().unwrap(); + let mut process_sampled_packets_us_hist = + self.process_sampled_packets_us_hist.lock().unwrap(); datapoint_info!( name, ( @@ -456,6 +457,7 @@ impl StreamStats { i64 ), ); + process_sampled_packets_us_hist.clear(); } } From 3b2565cae603853f049daf1cda3d3ffd5d9716fd Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Sat, 2 Mar 2024 03:28:32 -0800 Subject: [PATCH 16/42] Fixed a rebase issue --- accounts-db/src/accounts.rs | 29 +++--- accounts-db/src/accounts_db.rs | 35 +++---- banks-server/src/banks_server.rs | 10 +- core/src/banking_stage/consumer.rs | 21 +++-- .../forward_packet_batches_by_accounts.rs | 8 +- .../banking_stage/latest_unprocessed_votes.rs | 3 +- core/src/banking_stage/qos_service.rs | 12 ++- core/src/banking_stage/scheduler_messages.rs | 4 +- .../prio_graph_scheduler.rs | 26 +++--- .../scheduler_controller.rs | 27 ++++-- .../transaction_state.rs | 4 +- .../unprocessed_transaction_storage.rs | 46 ++++++---- entry/src/entry.rs | 27 ++++-- ledger/src/blockstore.rs | 2 +- ledger/src/blockstore_processor.rs | 16 ++-- ledger/src/token_balances.rs | 6 +- rpc/src/rpc.rs | 13 ++- rpc/src/transaction_status_service.rs | 25 +++-- runtime/src/bank.rs | 91 +++++++++++-------- runtime/src/bank_utils.rs | 11 ++- runtime/src/installed_scheduler_pool.rs | 8 +- runtime/src/prioritization_fee_cache.rs | 15 ++- runtime/src/transaction_batch.rs | 8 +- sdk/src/transaction/sanitized.rs | 17 ++++ svm/src/account_loader.rs | 6 +- svm/src/transaction_processor.rs | 13 +-- unified-scheduler-logic/src/lib.rs | 8 +- unified-scheduler-pool/src/lib.rs | 8 +- 28 files changed, 303 insertions(+), 196 deletions(-) diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 33a57d56461c78..5702353f9fc142 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -23,7 +23,9 @@ use { nonce_info::{NonceFull, NonceInfo}, pubkey::Pubkey, slot_hashes::SlotHashes, - transaction::{Result, SanitizedTransaction, TransactionAccountLocks, TransactionError}, + transaction::{ + ExtendedSanitizedTransaction, Result, TransactionAccountLocks, TransactionError, + }, transaction_context::TransactionAccount, }, solana_svm::{ @@ -31,10 +33,7 @@ use { }, std::{ cmp::Reverse, - collections::{ - hash_map::{self}, - BinaryHeap, HashMap, HashSet, - }, + collections::{hash_map, BinaryHeap, HashMap, HashSet}, ops::RangeBounds, sync::{ atomic::{AtomicUsize, Ordering}, @@ -577,11 +576,11 @@ impl Accounts { #[allow(clippy::needless_collect)] pub fn lock_accounts<'a>( &self, - txs: impl Iterator, + txs: impl Iterator, tx_account_lock_limit: usize, ) -> Vec> { let tx_account_locks_results: Vec> = txs - .map(|tx| tx.get_account_locks(tx_account_lock_limit)) + .map(|tx| tx.transaction.get_account_locks(tx_account_lock_limit)) .collect(); self.lock_accounts_inner(tx_account_locks_results) } @@ -590,14 +589,14 @@ impl Accounts { #[allow(clippy::needless_collect)] pub fn lock_accounts_with_results<'a>( &self, - txs: impl Iterator, + txs: impl Iterator, results: impl Iterator>, tx_account_lock_limit: usize, ) -> Vec> { let tx_account_locks_results: Vec> = txs .zip(results) .map(|(tx, result)| match result { - Ok(()) => tx.get_account_locks(tx_account_lock_limit), + Ok(()) => tx.transaction.get_account_locks(tx_account_lock_limit), Err(err) => Err(err), }) .collect(); @@ -627,11 +626,11 @@ impl Accounts { #[allow(clippy::needless_collect)] pub fn unlock_accounts<'a>( &self, - txs_and_results: impl Iterator)>, + txs_and_results: impl Iterator)>, ) { let keys: Vec<_> = txs_and_results .filter(|(_, res)| res.is_ok()) - .map(|(tx, _)| tx.get_account_locks_unchecked()) + .map(|(tx, _)| tx.transaction.get_account_locks_unchecked()) .collect(); if keys.is_empty() { return; @@ -650,7 +649,7 @@ impl Accounts { pub fn store_cached( &self, slot: Slot, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], res: &[TransactionExecutionResult], loaded: &mut [TransactionLoadResult], durable_nonce: &DurableNonce, @@ -677,14 +676,14 @@ impl Accounts { #[allow(clippy::too_many_arguments)] fn collect_accounts_to_store<'a>( &self, - txs: &'a [SanitizedTransaction], + txs: &'a [ExtendedSanitizedTransaction], execution_results: &'a [TransactionExecutionResult], load_results: &'a mut [TransactionLoadResult], durable_nonce: &DurableNonce, lamports_per_signature: u64, ) -> ( Vec<(&'a Pubkey, &'a AccountSharedData)>, - Vec>, + Vec>, ) { let mut accounts = Vec::with_capacity(load_results.len()); let mut transactions = Vec::with_capacity(load_results.len()); @@ -712,7 +711,7 @@ impl Accounts { } }; - let message = tx.message(); + let message = tx.transaction.message(); let loaded_transaction = tx_load_result.as_mut().unwrap(); let mut fee_payer_index = None; for (i, (address, account)) in (0..message.account_keys().len()) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index eab5ca33af417c..648be7fd5b167b 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -93,7 +93,7 @@ use { rent_collector::RentCollector, saturating_add_assign, timing::AtomicInterval, - transaction::SanitizedTransaction, + transaction::ExtendedSanitizedTransaction, }, std::{ borrow::{Borrow, Cow}, @@ -6373,7 +6373,7 @@ impl AccountsDb { &self, slot: Slot, accounts_and_meta_to_store: &impl StorableAccounts<'b, T>, - txn_iter: Box> + 'a>, + txn_iter: Box> + 'a>, mut write_version_producer: P, ) -> Vec where @@ -6391,7 +6391,7 @@ impl AccountsDb { self.notify_account_at_accounts_update( slot, &account, - txn, + &txn.map(|txn| &txn.transaction), accounts_and_meta_to_store.pubkey(i), &mut write_version_producer, ); @@ -6423,7 +6423,7 @@ impl AccountsDb { hashes: Option>>, mut write_version_producer: P, store_to: &StoreTo, - transactions: Option<&[Option<&'a SanitizedTransaction>]>, + transactions: Option<&[Option<&'a ExtendedSanitizedTransaction>]>, ) -> Vec { let mut calc_stored_meta_time = Measure::start("calc_stored_meta"); let slot = accounts.target_slot(); @@ -6438,14 +6438,15 @@ impl AccountsDb { match store_to { StoreTo::Cache => { - let txn_iter: Box>> = - match transactions { - Some(transactions) => { - assert_eq!(transactions.len(), accounts.len()); - Box::new(transactions.iter()) - } - None => Box::new(std::iter::repeat(&None).take(accounts.len())), - }; + let txn_iter: Box< + dyn std::iter::Iterator>, + > = match transactions { + Some(transactions) => { + assert_eq!(transactions.len(), accounts.len()); + Box::new(transactions.iter()) + } + None => Box::new(std::iter::repeat(&None).take(accounts.len())), + }; self.write_accounts_to_cache(slot, accounts, txn_iter, write_version_producer) } @@ -8125,7 +8126,7 @@ impl AccountsDb { pub fn store_cached<'a, T: ReadableAccount + Sync + ZeroLamport + 'a>( &self, accounts: impl StorableAccounts<'a, T>, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [Option<&'a ExtendedSanitizedTransaction>]>, ) { self.store( accounts, @@ -8142,7 +8143,7 @@ impl AccountsDb { >( &self, accounts: impl StorableAccounts<'a, T>, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [Option<&'a ExtendedSanitizedTransaction>]>, ) { self.store( accounts, @@ -8170,7 +8171,7 @@ impl AccountsDb { &self, accounts: impl StorableAccounts<'a, T>, store_to: &StoreTo, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [Option<&'a ExtendedSanitizedTransaction>]>, reclaim: StoreReclaims, update_index_thread_selection: UpdateIndexThreadSelection, ) { @@ -8345,7 +8346,7 @@ impl AccountsDb { accounts: impl StorableAccounts<'a, T>, hashes: Option>>, store_to: &StoreTo, - transactions: Option<&'a [Option<&'a SanitizedTransaction>]>, + transactions: Option<&'a [Option<&'a ExtendedSanitizedTransaction>]>, reclaim: StoreReclaims, update_index_thread_selection: UpdateIndexThreadSelection, ) { @@ -8400,7 +8401,7 @@ impl AccountsDb { write_version_producer: Option>>, store_to: &StoreTo, reset_accounts: bool, - transactions: Option<&[Option<&SanitizedTransaction>]>, + transactions: Option<&[Option<&ExtendedSanitizedTransaction>]>, reclaim: StoreReclaims, update_index_thread_selection: UpdateIndexThreadSelection, ) -> StoreAccountsTiming { diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index b3028c0132ed48..973c591e622edb 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -23,7 +23,10 @@ use { message::{Message, SanitizedMessage}, pubkey::Pubkey, signature::Signature, - transaction::{self, MessageHash, SanitizedTransaction, VersionedTransaction}, + transaction::{ + self, ExtendedSanitizedTransaction, MessageHash, SanitizedTransaction, + VersionedTransaction, + }, }, solana_send_transaction_service::{ send_transaction_service::{SendTransactionService, TransactionInfo}, @@ -194,7 +197,10 @@ fn simulate_transaction( units_consumed, return_data, inner_instructions, - } = bank.simulate_transaction_unchecked(&sanitized_transaction, false); + } = bank.simulate_transaction_unchecked( + &ExtendedSanitizedTransaction::from(sanitized_transaction), + false, + ); let simulation_details = TransactionSimulationDetails { logs, diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 9e9475f7678e3c..0c5ae73c5ba651 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -29,7 +29,7 @@ use { message::SanitizedMessage, saturating_add_assign, timing::timestamp, - transaction::{self, AddressLoader, SanitizedTransaction, TransactionError}, + transaction::{self, AddressLoader, ExtendedSanitizedTransaction, TransactionError}, }, solana_svm::{ account_loader::{validate_fee_payer, TransactionCheckResult}, @@ -242,7 +242,7 @@ impl Consumer { &self, bank: &Arc, bank_creation_time: &Instant, - sanitized_transactions: &[SanitizedTransaction], + sanitized_transactions: &[ExtendedSanitizedTransaction], banking_stage_stats: &BankingStageStats, slot_metrics_tracker: &mut LeaderSlotMetricsTracker, ) -> ProcessTransactionsSummary { @@ -298,7 +298,7 @@ impl Consumer { &self, bank: &Arc, bank_creation_time: &Instant, - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], ) -> ProcessTransactionsSummary { let mut chunk_start = 0; let mut all_retryable_tx_indexes = vec![]; @@ -432,7 +432,7 @@ impl Consumer { pub fn process_and_record_transactions( &self, bank: &Arc, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], chunk_offset: usize, ) -> ProcessTransactionBatchOutput { let mut error_counters = TransactionErrorMetrics::default(); @@ -460,7 +460,7 @@ impl Consumer { pub fn process_and_record_aged_transactions( &self, bank: &Arc, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], max_slot_ages: &[Slot], ) -> ProcessTransactionBatchOutput { // Need to filter out transactions since they were sanitized earlier. @@ -472,7 +472,7 @@ impl Consumer { // Re-sanitized transaction should be equal to the original transaction, // but whether it will pass sanitization needs to be checked. let resanitized_tx = - bank.fully_verify_transaction(tx.to_versioned_transaction())?; + bank.fully_verify_transaction(tx.transaction.to_versioned_transaction())?; if resanitized_tx != *tx { // Sanitization before/after epoch give different transaction data - do not execute. return Err(TransactionError::ResanitizationNeeded); @@ -480,7 +480,7 @@ impl Consumer { } else { // 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(); + let lookup_tables = tx.transaction.message().message_address_table_lookups(); if !lookup_tables.is_empty() { bank.load_addresses(lookup_tables)?; } @@ -493,7 +493,7 @@ impl Consumer { fn process_and_record_transactions_with_pre_results( &self, bank: &Arc, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], chunk_offset: usize, pre_results: impl Iterator>, ) -> ProcessTransactionBatchOutput { @@ -609,6 +609,7 @@ impl Consumer { .filter_map(|transaction| { let round_compute_unit_price_enabled = false; // TODO get from working_bank.feature_set transaction + .transaction .get_compute_budget_details(round_compute_unit_price_enabled) .map(|details| details.compute_unit_price) }) @@ -647,7 +648,7 @@ impl Consumer { .zip(batch.sanitized_transactions()) .filter_map(|(execution_result, tx)| { if execution_result.was_executed() { - Some(tx.to_versioned_transaction()) + Some(tx.transaction.to_versioned_transaction()) } else { None } @@ -811,7 +812,7 @@ impl Consumer { /// * `pending_indexes` - identifies which indexes in the `transactions` list are still pending fn filter_pending_packets_from_pending_txs( bank: &Bank, - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], pending_indexes: &[usize], ) -> Vec { let filter = diff --git a/core/src/banking_stage/forward_packet_batches_by_accounts.rs b/core/src/banking_stage/forward_packet_batches_by_accounts.rs index 54efb27ce56129..ab8f52820e8eac 100644 --- a/core/src/banking_stage/forward_packet_batches_by_accounts.rs +++ b/core/src/banking_stage/forward_packet_batches_by_accounts.rs @@ -6,7 +6,7 @@ use { cost_tracker::{CostTracker, CostTrackerError}, }, solana_perf::packet::Packet, - solana_sdk::{feature_set::FeatureSet, transaction::SanitizedTransaction}, + solana_sdk::{feature_set::FeatureSet, transaction::ExtendedSanitizedTransaction}, std::sync::Arc, }; @@ -58,11 +58,11 @@ impl ForwardBatch { fn try_add( &mut self, - sanitized_transaction: &SanitizedTransaction, + sanitized_transaction: &ExtendedSanitizedTransaction, immutable_packet: Arc, feature_set: &FeatureSet, ) -> Result { - let tx_cost = CostModel::calculate_cost(sanitized_transaction, feature_set); + let tx_cost = CostModel::calculate_cost(&sanitized_transaction.transaction, feature_set); let res = self.cost_tracker.try_add(&tx_cost); if res.is_ok() { self.forwardable_packets.push(immutable_packet); @@ -112,7 +112,7 @@ impl ForwardPacketBatchesByAccounts { /// packets are filled into first available 'batch' that have space to fit it. pub fn try_add_packet( &mut self, - sanitized_transaction: &SanitizedTransaction, + sanitized_transaction: &ExtendedSanitizedTransaction, immutable_packet: Arc, feature_set: &FeatureSet, ) -> bool { diff --git a/core/src/banking_stage/latest_unprocessed_votes.rs b/core/src/banking_stage/latest_unprocessed_votes.rs index a62e5bf9b3e455..fecec111bd660c 100644 --- a/core/src/banking_stage/latest_unprocessed_votes.rs +++ b/core/src/banking_stage/latest_unprocessed_votes.rs @@ -11,6 +11,7 @@ use { clock::{Slot, UnixTimestamp}, program_utils::limited_deserialize, pubkey::Pubkey, + transaction::ExtendedSanitizedTransaction, }, solana_vote_program::vote_instruction::VoteInstruction, std::{ @@ -286,7 +287,7 @@ impl LatestUnprocessedVotes { ) { if forward_packet_batches_by_accounts.try_add_packet( - &sanitized_vote_transaction, + &ExtendedSanitizedTransaction::from(sanitized_vote_transaction), deserialized_vote_packet, &bank.feature_set, ) { diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 8c1507ae3fb91c..5532d36eb39ecd 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -12,7 +12,7 @@ use { clock::Slot, feature_set::FeatureSet, saturating_add_assign, - transaction::{self, SanitizedTransaction, TransactionError}, + transaction::{self, ExtendedSanitizedTransaction, TransactionError}, }, std::sync::atomic::{AtomicU64, Ordering}, }; @@ -40,7 +40,7 @@ impl QosService { pub fn select_and_accumulate_transaction_costs( &self, bank: &Bank, - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], pre_results: impl Iterator>, ) -> (Vec>, usize) { let transaction_costs = @@ -67,13 +67,15 @@ impl QosService { fn compute_transaction_costs<'a>( &self, feature_set: &FeatureSet, - transactions: impl Iterator, + transactions: impl Iterator, pre_results: impl Iterator>, ) -> Vec> { let mut compute_cost_time = Measure::start("compute_cost_time"); let txs_costs: Vec<_> = transactions .zip(pre_results) - .map(|(tx, pre_result)| pre_result.map(|()| CostModel::calculate_cost(tx, feature_set))) + .map(|(tx, pre_result)| { + pre_result.map(|()| CostModel::calculate_cost(&tx.transaction, feature_set)) + }) .collect(); compute_cost_time.stop(); self.metrics @@ -92,7 +94,7 @@ impl QosService { /// and a count of the number of transactions that would fit in the block fn select_transactions_per_cost<'a>( &self, - transactions: impl Iterator, + transactions: impl Iterator, transactions_costs: impl Iterator>, bank: &Bank, ) -> (Vec>, usize) { diff --git a/core/src/banking_stage/scheduler_messages.rs b/core/src/banking_stage/scheduler_messages.rs index 172087e2cf8e82..359f4cac710b71 100644 --- a/core/src/banking_stage/scheduler_messages.rs +++ b/core/src/banking_stage/scheduler_messages.rs @@ -1,6 +1,6 @@ use { super::immutable_deserialized_packet::ImmutableDeserializedPacket, - solana_sdk::{clock::Slot, transaction::SanitizedTransaction}, + solana_sdk::{clock::Slot, transaction::ExtendedSanitizedTransaction}, std::{fmt::Display, sync::Arc}, }; @@ -41,7 +41,7 @@ impl Display for TransactionId { pub struct ConsumeWork { pub batch_id: TransactionBatchId, pub ids: Vec, - pub transactions: Vec, + pub transactions: Vec, pub max_age_slots: Vec, } 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 d983fcf4d163c3..95408800852b84 100644 --- a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs +++ b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs @@ -18,7 +18,7 @@ use { solana_measure::measure_us, solana_sdk::{ pubkey::Pubkey, saturating_add_assign, slot_history::Slot, - transaction::SanitizedTransaction, + transaction::ExtendedSanitizedTransaction, }, }; @@ -64,8 +64,8 @@ impl PrioGraphScheduler { pub(crate) fn schedule( &mut self, container: &mut TransactionStateContainer, - pre_graph_filter: impl Fn(&[&SanitizedTransaction], &mut [bool]), - pre_lock_filter: impl Fn(&SanitizedTransaction) -> bool, + pre_graph_filter: impl Fn(&[&ExtendedSanitizedTransaction], &mut [bool]), + pre_lock_filter: impl Fn(&ExtendedSanitizedTransaction) -> bool, ) -> Result { let num_threads = self.consume_work_senders.len(); let mut batches = Batches::new(num_threads); @@ -161,15 +161,15 @@ impl PrioGraphScheduler { } // Check if this transaction conflicts with any blocked transactions - if !blocking_locks.check_locks(transaction.message()) { - blocking_locks.take_locks(transaction.message()); + if !blocking_locks.check_locks(transaction.transaction.message()) { + blocking_locks.take_locks(transaction.transaction.message()); unschedulable_ids.push(id); saturating_add_assign!(num_unschedulable, 1); continue; } // Schedule the transaction if it can be. - let transaction_locks = transaction.get_account_locks_unchecked(); + let transaction_locks = transaction.transaction.get_account_locks_unchecked(); let Some(thread_id) = self.account_locks.try_lock_accounts( transaction_locks.writable.into_iter(), transaction_locks.readonly.into_iter(), @@ -182,7 +182,7 @@ impl PrioGraphScheduler { ) }, ) else { - blocking_locks.take_locks(transaction.message()); + blocking_locks.take_locks(transaction.transaction.message()); unschedulable_ids.push(id); saturating_add_assign!(num_unschedulable, 1); continue; @@ -329,11 +329,11 @@ impl PrioGraphScheduler { fn complete_batch( &mut self, batch_id: TransactionBatchId, - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], ) { let thread_id = self.in_flight_tracker.complete_batch(batch_id); for transaction in transactions { - let account_locks = transaction.get_account_locks_unchecked(); + let account_locks = transaction.transaction.get_account_locks_unchecked(); self.account_locks.unlock_accounts( account_locks.writable.into_iter(), account_locks.readonly.into_iter(), @@ -392,7 +392,7 @@ impl PrioGraphScheduler { /// on `ThreadAwareAccountLocks::try_lock_accounts`. fn select_thread( thread_set: ThreadSet, - batches_per_thread: &[Vec], + batches_per_thread: &[Vec], in_flight_per_thread: &[usize], ) -> ThreadId { thread_set @@ -412,7 +412,7 @@ impl PrioGraphScheduler { fn get_transaction_account_access( transaction: &SanitizedTransactionTTL, ) -> impl Iterator + '_ { - let message = transaction.transaction.message(); + let message = transaction.transaction.transaction.message(); message .account_keys() .iter() @@ -442,7 +442,7 @@ pub(crate) struct SchedulingSummary { struct Batches { ids: Vec>, - transactions: Vec>, + transactions: Vec>, max_age_slots: Vec>, total_cus: Vec, } @@ -462,7 +462,7 @@ impl Batches { thread_id: ThreadId, ) -> ( Vec, - Vec, + Vec, Vec, u64, ) { diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index 12e8f7bf8bf0bf..600c7ef6c3f3a3 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -31,7 +31,7 @@ use { }, fee::FeeBudgetLimits, saturating_add_assign, - transaction::SanitizedTransaction, + transaction::{ExtendedSanitizedTransaction, SanitizedTransaction}, }, solana_svm::transaction_error_metrics::TransactionErrorMetrics, std::{ @@ -189,7 +189,11 @@ impl SchedulerController { Ok(()) } - fn pre_graph_filter(transactions: &[&SanitizedTransaction], results: &mut [bool], bank: &Bank) { + fn pre_graph_filter( + transactions: &[&ExtendedSanitizedTransaction], + results: &mut [bool], + bank: &Bank, + ) { let lock_results = vec![Ok(()); transactions.len()]; let mut error_counters = TransactionErrorMetrics::default(); let check_results = bank.check_transactions( @@ -204,7 +208,11 @@ impl SchedulerController { .zip(transactions) .map(|((result, _nonce, _lamports), tx)| { result?; // if there's already error do nothing - Consumer::check_fee_payer_unlocked(bank, tx.message(), &mut error_counters) + Consumer::check_fee_payer_unlocked( + bank, + tx.transaction.message(), + &mut error_counters, + ) }) .collect(); @@ -387,7 +395,12 @@ impl SchedulerController { }) .filter_map(|tx| { process_compute_budget_instructions(tx.message().program_instructions_iter()) - .map(|compute_budget| (tx, compute_budget.into())) + .map(|compute_budget| { + ( + ExtendedSanitizedTransaction::from(tx), + compute_budget.into(), + ) + }) .ok() }) .unzip(); @@ -478,13 +491,13 @@ impl SchedulerController { /// Any difference in the prioritization is negligible for /// the current transaction costs. fn calculate_priority_and_cost( - transaction: &SanitizedTransaction, + transaction: &ExtendedSanitizedTransaction, fee_budget_limits: &FeeBudgetLimits, bank: &Bank, ) -> (u64, u64) { - let cost = CostModel::calculate_cost(transaction, &bank.feature_set).sum(); + let cost = CostModel::calculate_cost(&transaction.transaction, &bank.feature_set).sum(); let fee = bank.fee_structure.calculate_fee( - transaction.message(), + transaction.transaction.message(), 5_000, // this just needs to be non-zero fee_budget_limits, bank.feature_set diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state.rs b/core/src/banking_stage/transaction_scheduler/transaction_state.rs index 727140545ab656..7ae094258a9bad 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state.rs @@ -1,8 +1,8 @@ -use solana_sdk::{clock::Slot, transaction::SanitizedTransaction}; +use solana_sdk::{clock::Slot, transaction::ExtendedSanitizedTransaction}; /// Simple wrapper type to tie a sanitized transaction to max age slot. pub(crate) struct SanitizedTransactionTTL { - pub(crate) transaction: SanitizedTransaction, + pub(crate) transaction: ExtendedSanitizedTransaction, pub(crate) max_age_slot: Slot, } diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 52706f8c2bf63b..7c50e045b89f62 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -20,8 +20,11 @@ use { solana_measure::{measure, measure_us}, solana_runtime::bank::Bank, solana_sdk::{ - clock::FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, feature_set::FeatureSet, hash::Hash, - saturating_add_assign, transaction::SanitizedTransaction, + clock::FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, + feature_set::FeatureSet, + hash::Hash, + saturating_add_assign, + transaction::{ExtendedSanitizedTransaction, SanitizedTransaction}, }, solana_svm::transaction_error_metrics::TransactionErrorMetrics, std::{ @@ -135,7 +138,7 @@ fn filter_processed_packets<'a, F>( pub struct ConsumeScannerPayload<'a> { pub reached_end_of_slot: bool, pub account_locks: ReadWriteAccountSet, - pub sanitized_transactions: Vec, + pub sanitized_transactions: Vec, pub slot_metrics_tracker: &'a mut LeaderSlotMetricsTracker, pub message_hash_to_transaction: &'a mut HashMap, pub error_counters: TransactionErrorMetrics, @@ -208,7 +211,9 @@ fn consume_scan_should_process_packet( return ProcessingDecision::Later; } - payload.sanitized_transactions.push(sanitized_transaction); + payload + .sanitized_transactions + .push(ExtendedSanitizedTransaction::from(sanitized_transaction)); ProcessingDecision::Now } else { payload @@ -635,7 +640,7 @@ impl ThreadLocalUnprocessedPackets { (sanitized_transactions, transaction_to_packet_indexes), packet_conversion_time, ): ( - (Vec, Vec), + (Vec, Vec), _, ) = measure!( self.sanitize_unforwarded_packets( @@ -762,18 +767,25 @@ impl ThreadLocalUnprocessedPackets { packets_to_process: &[Arc], bank: &Bank, total_dropped_packets: &mut usize, - ) -> (Vec, Vec) { + ) -> (Vec, Vec) { // Get ref of ImmutableDeserializedPacket let deserialized_packets = packets_to_process.iter().map(|p| &**p); - let (transactions, transaction_to_packet_indexes): (Vec, Vec) = - deserialized_packets - .enumerate() - .filter_map(|(packet_index, deserialized_packet)| { - deserialized_packet - .build_sanitized_transaction(&bank.feature_set, bank.vote_only_bank(), bank) - .map(|transaction| (transaction, packet_index)) - }) - .unzip(); + let (transactions, transaction_to_packet_indexes): ( + Vec, + Vec, + ) = deserialized_packets + .enumerate() + .filter_map(|(packet_index, deserialized_packet)| { + deserialized_packet + .build_sanitized_transaction(&bank.feature_set, bank.vote_only_bank(), bank) + .map(|transaction| { + ( + ExtendedSanitizedTransaction::from(transaction), + packet_index, + ) + }) + }) + .unzip(); let filtered_count = packets_to_process.len().saturating_sub(transactions.len()); saturating_add_assign!(*total_dropped_packets, filtered_count); @@ -783,7 +795,7 @@ impl ThreadLocalUnprocessedPackets { /// Checks sanitized transactions against bank, returns valid transaction indexes fn filter_invalid_transactions( - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], bank: &Bank, total_dropped_packets: &mut usize, ) -> Vec { @@ -821,7 +833,7 @@ impl ThreadLocalUnprocessedPackets { fn add_filtered_packets_to_forward_buffer( forward_buffer: &mut ForwardPacketBatchesByAccounts, packets_to_process: &[Arc], - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], transaction_to_packet_indexes: &[usize], forwardable_transaction_indexes: &[usize], total_dropped_packets: &mut usize, diff --git a/entry/src/entry.rs b/entry/src/entry.rs index af3fdca9518e83..0029148dcee18a 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -27,7 +27,7 @@ use { packet::Meta, timing, transaction::{ - Result, SanitizedTransaction, Transaction, TransactionError, + ExtendedSanitizedTransaction, Result, Transaction, TransactionError, TransactionVerificationMode, VersionedTransaction, }, }, @@ -163,7 +163,7 @@ impl From<&Entry> for EntrySummary { /// Typed entry to distinguish between transaction and tick entries pub enum EntryType { - Transactions(Vec), + Transactions(Vec), Tick(Hash), } @@ -405,7 +405,7 @@ impl EntryVerificationState { pub fn verify_transactions( entries: Vec, - verify: Arc Result + Send + Sync>, + verify: Arc Result + Send + Sync>, ) -> Result> { PAR_THREAD_POOL.install(|| { entries @@ -432,7 +432,10 @@ pub fn start_verify_transactions( skip_verification: bool, verify_recyclers: VerifyRecyclers, verify: Arc< - dyn Fn(VersionedTransaction, TransactionVerificationMode) -> Result + dyn Fn( + VersionedTransaction, + TransactionVerificationMode, + ) -> Result + Send + Sync, >, @@ -469,7 +472,10 @@ fn start_verify_transactions_cpu( entries: Vec, skip_verification: bool, verify: Arc< - dyn Fn(VersionedTransaction, TransactionVerificationMode) -> Result + dyn Fn( + VersionedTransaction, + TransactionVerificationMode, + ) -> Result + Send + Sync, >, @@ -498,13 +504,16 @@ fn start_verify_transactions_gpu( entries: Vec, verify_recyclers: VerifyRecyclers, verify: Arc< - dyn Fn(VersionedTransaction, TransactionVerificationMode) -> Result + dyn Fn( + VersionedTransaction, + TransactionVerificationMode, + ) -> Result + Send + Sync, >, ) -> Result { let verify_func = { - move |versioned_tx: VersionedTransaction| -> Result { + move |versioned_tx: VersionedTransaction| -> Result { verify( versioned_tx, TransactionVerificationMode::HashAndVerifyPrecompiles, @@ -514,7 +523,7 @@ fn start_verify_transactions_gpu( let entries = verify_transactions(entries, Arc::new(verify_func))?; - let entry_txs: Vec<&SanitizedTransaction> = entries + let entry_txs: Vec<&ExtendedSanitizedTransaction> = entries .iter() .filter_map(|entry_type| match entry_type { EntryType::Tick(_) => None, @@ -552,7 +561,7 @@ fn start_verify_transactions_gpu( } let entry_tx_iter = slice .into_par_iter() - .map(|tx| tx.to_versioned_transaction()); + .map(|tx| tx.transaction.to_versioned_transaction()); let res = packet_batch .par_iter_mut() diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index f15976abdb241b..24f94f762b137a 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -3095,7 +3095,7 @@ impl Blockstore { // Attempt to verify transaction and load addresses from the current bank, // or manually scan the transaction for addresses if the transaction. if let Ok(tx) = bank.fully_verify_transaction(tx.clone()) { - add_to_set(&result, tx.message().account_keys().iter()); + add_to_set(&result, tx.transaction.message().account_keys().iter()); } else { add_to_set(&result, tx.message.static_account_keys()); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index e4ae5f368b2afd..d478c8fa6dfa1b 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -53,7 +53,7 @@ use { signature::{Keypair, Signature}, timing, transaction::{ - Result, SanitizedTransaction, TransactionError, TransactionVerificationMode, + ExtendedSanitizedTransaction, Result, TransactionError, TransactionVerificationMode, VersionedTransaction, }, }, @@ -120,7 +120,7 @@ fn get_first_error( { if let Err(ref err) = result { if first_err.is_none() { - first_err = Some((result.clone(), *transaction.signature())); + first_err = Some((result.clone(), *transaction.transaction.signature())); } warn!( "Unexpected validator error: {:?}, transaction: {:?}", @@ -379,7 +379,7 @@ fn schedule_batches_for_execution( fn rebatch_transactions<'a>( lock_results: &'a [Result<()>], bank: &'a Arc, - sanitized_txs: &'a [SanitizedTransaction], + sanitized_txs: &'a [ExtendedSanitizedTransaction], start: usize, end: usize, transaction_indexes: &'a [usize], @@ -427,7 +427,7 @@ fn rebatch_and_execute_batches( let tx_costs = sanitized_txs .iter() .map(|tx| { - let tx_cost = CostModel::calculate_cost(tx, &bank.feature_set); + let tx_cost = CostModel::calculate_cost(&tx.transaction, &bank.feature_set); let cost = tx_cost.sum(); minimal_tx_cost = std::cmp::min(minimal_tx_cost, cost); total_cost = total_cost.saturating_add(cost); @@ -508,7 +508,7 @@ pub fn process_entries_for_tests( ) -> Result<()> { let verify_transaction = { let bank = bank.clone_with_scheduler(); - move |versioned_tx: VersionedTransaction| -> Result { + move |versioned_tx: VersionedTransaction| -> Result { bank.verify_transaction(versioned_tx, TransactionVerificationMode::FullVerification) } }; @@ -1287,7 +1287,7 @@ fn confirm_slot_entries( let bank = bank.clone_with_scheduler(); move |versioned_tx: VersionedTransaction, verification_mode: TransactionVerificationMode| - -> Result { + -> Result { bank.verify_transaction(versioned_tx, verification_mode) } }; @@ -1834,7 +1834,7 @@ pub enum TransactionStatusMessage { pub struct TransactionStatusBatch { pub bank: Arc, - pub transactions: Vec, + pub transactions: Vec, pub execution_results: Vec>, pub balances: TransactionBalancesSet, pub token_balances: TransactionTokenBalancesSet, @@ -1851,7 +1851,7 @@ impl TransactionStatusSender { pub fn send_transaction_status_batch( &self, bank: Arc, - transactions: Vec, + transactions: Vec, execution_results: Vec, balances: TransactionBalancesSet, token_balances: TransactionTokenBalancesSet, diff --git a/ledger/src/token_balances.rs b/ledger/src/token_balances.rs index 204bd4335972aa..ff133127a14b4a 100644 --- a/ledger/src/token_balances.rs +++ b/ledger/src/token_balances.rs @@ -43,13 +43,15 @@ pub fn collect_token_balances( let mut collect_time = Measure::start("collect_token_balances"); for transaction in batch.sanitized_transactions() { - let account_keys = transaction.message().account_keys(); + let account_keys = transaction.transaction.message().account_keys(); let has_token_program = account_keys.iter().any(is_known_spl_token_id); let mut transaction_balances: Vec = vec![]; if has_token_program { for (index, account_id) in account_keys.iter().enumerate() { - if transaction.message().is_invoked(index) || is_known_spl_token_id(account_id) { + if transaction.transaction.message().is_invoked(index) + || is_known_spl_token_id(account_id) + { continue; } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 41b26e5fa1e2c2..dc343a2cf05dc2 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3252,7 +3252,10 @@ pub mod rpc_accounts_scan { pub mod rpc_full { use { super::*, - solana_sdk::message::{SanitizedVersionedMessage, VersionedMessage}, + solana_sdk::{ + message::{SanitizedVersionedMessage, VersionedMessage}, + transaction::ExtendedSanitizedTransaction, + }, solana_transaction_status::UiInnerInstructions, }; #[rpc] @@ -3665,7 +3668,8 @@ pub mod rpc_full { units_consumed, return_data, inner_instructions: _, // Always `None` due to `enable_cpi_recording = false` - } = preflight_bank.simulate_transaction(&transaction, false) + } = preflight_bank + .simulate_transaction(&ExtendedSanitizedTransaction::from(transaction), false) { match err { TransactionError::BlockhashNotFound => { @@ -3741,8 +3745,9 @@ pub mod rpc_full { } let transaction = sanitize_transaction(unsanitized_tx, bank)?; + let transaction = ExtendedSanitizedTransaction::from(transaction); if sig_verify { - verify_transaction(&transaction, &bank.feature_set)?; + verify_transaction(&transaction.transaction, &bank.feature_set)?; } let TransactionSimulationResult { @@ -3754,7 +3759,7 @@ pub mod rpc_full { inner_instructions, } = bank.simulate_transaction(&transaction, enable_cpi_recording); - let account_keys = transaction.message().account_keys(); + let account_keys = transaction.transaction.message().account_keys(); let number_of_accounts = account_keys.len(); let accounts = if let Some(config_accounts) = config_accounts { diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 8730fb2ed0f3d8..29a20542379ba3 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -110,15 +110,16 @@ impl TransactionStatusService { } Some(DurableNonceFee::Invalid) => None, None => bank.get_lamports_per_signature_for_blockhash( - transaction.message().recent_blockhash(), + transaction.transaction.message().recent_blockhash(), ), } .expect("lamports_per_signature must be available"); let fee = bank.get_fee_for_message_with_lamports_per_signature( - transaction.message(), + transaction.transaction.message(), lamports_per_signature, ); - let tx_account_locks = transaction.get_account_locks_unchecked(); + let tx_account_locks = + transaction.transaction.get_account_locks_unchecked(); let inner_instructions = inner_instructions.map(|inner_instructions| { map_inner_instructions(inner_instructions).collect() @@ -138,7 +139,7 @@ impl TransactionStatusService { }) .collect(), ); - let loaded_addresses = transaction.get_loaded_addresses(); + let loaded_addresses = transaction.transaction.get_loaded_addresses(); let mut transaction_status_meta = TransactionStatusMeta { status, fee, @@ -158,9 +159,9 @@ impl TransactionStatusService { transaction_notifier.notify_transaction( slot, transaction_index, - transaction.signature(), + transaction.transaction.signature(), &transaction_status_meta, - &transaction, + &transaction.transaction, ); } @@ -172,16 +173,22 @@ impl TransactionStatusService { } if enable_rpc_transaction_history { - if let Some(memos) = extract_and_fmt_memos(transaction.message()) { + if let Some(memos) = + extract_and_fmt_memos(transaction.transaction.message()) + { blockstore - .write_transaction_memos(transaction.signature(), slot, memos) + .write_transaction_memos( + transaction.transaction.signature(), + slot, + memos, + ) .expect("Expect database write to succeed: TransactionMemos"); } blockstore .write_transaction_status( slot, - *transaction.signature(), + *transaction.transaction.signature(), tx_account_locks.writable, tx_account_locks.readonly, transaction_status_meta, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d1a1805d0d3a20..393291b6ea18fd 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -152,8 +152,9 @@ use { sysvar::{self, last_restart_slot::LastRestartSlot, Sysvar, SysvarId}, timing::years_as_slots, transaction::{ - self, MessageHash, Result, SanitizedTransaction, Transaction, TransactionError, - TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS, + self, ExtendedSanitizedTransaction, MessageHash, Result, SanitizedTransaction, + Transaction, TransactionError, TransactionVerificationMode, VersionedTransaction, + MAX_TX_ACCOUNT_LOCKS, }, transaction_context::{TransactionAccount, TransactionReturnData}, }, @@ -4081,7 +4082,7 @@ impl Bank { fn update_transaction_statuses( &self, - sanitized_txs: &[SanitizedTransaction], + sanitized_txs: &[ExtendedSanitizedTransaction], execution_results: &[TransactionExecutionResult], ) { let mut status_cache = self.status_cache.write().unwrap(); @@ -4091,8 +4092,8 @@ impl Bank { // Add the message hash to the status cache to ensure that this message // won't be processed again with a different signature. status_cache.insert( - tx.message().recent_blockhash(), - tx.message_hash(), + tx.transaction.message().recent_blockhash(), + tx.transaction.message_hash(), self.slot(), details.status.clone(), ); @@ -4100,8 +4101,8 @@ impl Bank { // can be queried by transaction signature over RPC. In the future, this should // only be added for API nodes because voting validators don't need to do this. status_cache.insert( - tx.message().recent_blockhash(), - tx.signature(), + tx.transaction.message().recent_blockhash(), + tx.transaction.signature(), self.slot(), details.status.clone(), ); @@ -4202,7 +4203,16 @@ impl Bank { pub fn prepare_entry_batch(&self, txs: Vec) -> Result { let sanitized_txs = txs .into_iter() - .map(|tx| SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self)) + .map(|tx| { + SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self).and_then( + |txn| { + Ok(ExtendedSanitizedTransaction { + transaction: txn, + start_time: None, + }) + }, + ) + }) .collect::>>()?; let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_results = self @@ -4219,7 +4229,7 @@ impl Bank { /// Prepare a locked transaction batch from a list of sanitized transactions. pub fn prepare_sanitized_batch<'a, 'b>( &'a self, - txs: &'b [SanitizedTransaction], + txs: &'b [ExtendedSanitizedTransaction], ) -> TransactionBatch<'a, 'b> { let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_results = self @@ -4233,7 +4243,7 @@ impl Bank { /// limited packing status pub fn prepare_sanitized_batch_with_results<'a, 'b>( &'a self, - transactions: &'b [SanitizedTransaction], + transactions: &'b [ExtendedSanitizedTransaction], transaction_results: impl Iterator>, ) -> TransactionBatch<'a, 'b> { // this lock_results could be: Ok, AccountInUse, WouldExceedBlockMaxLimit or WouldExceedAccountMaxLimit @@ -4249,10 +4259,11 @@ impl Bank { /// Prepare a transaction batch from a single transaction without locking accounts pub fn prepare_unlocked_batch_from_single_tx<'a>( &'a self, - transaction: &'a SanitizedTransaction, + transaction: &'a ExtendedSanitizedTransaction, ) -> TransactionBatch<'_, '_> { let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_result = transaction + .transaction .get_account_locks(tx_account_lock_limit) .map(|_| ()); let mut batch = TransactionBatch::new( @@ -4267,7 +4278,7 @@ impl Bank { /// Run transactions against a frozen bank without committing the results pub fn simulate_transaction( &self, - transaction: &SanitizedTransaction, + transaction: &ExtendedSanitizedTransaction, enable_cpi_recording: bool, ) -> TransactionSimulationResult { assert!(self.is_frozen(), "simulation bank must be frozen"); @@ -4279,10 +4290,10 @@ impl Bank { /// is frozen, enabling use in single-Bank test frameworks pub fn simulate_transaction_unchecked( &self, - transaction: &SanitizedTransaction, + transaction: &ExtendedSanitizedTransaction, enable_cpi_recording: bool, ) -> TransactionSimulationResult { - let account_keys = transaction.message().account_keys(); + let account_keys = transaction.transaction.message().account_keys(); let number_of_accounts = account_keys.len(); let account_overrides = self.get_account_overrides_for_simulation(&account_keys); let batch = self.prepare_unlocked_batch_from_single_tx(transaction); @@ -4395,7 +4406,7 @@ impl Bank { fn check_age( &self, - sanitized_txs: &[impl core::borrow::Borrow], + sanitized_txs: &[impl core::borrow::Borrow], lock_results: &[Result<()>], max_age: usize, error_counters: &mut TransactionErrorMetrics, @@ -4409,7 +4420,7 @@ impl Bank { .zip(lock_results) .map(|(tx, lock_res)| match lock_res { Ok(()) => self.check_transaction_age( - tx.borrow(), + &tx.borrow().transaction, max_age, &next_durable_nonce, &hash_queue, @@ -4461,7 +4472,7 @@ impl Bank { fn check_status_cache( &self, - sanitized_txs: &[impl core::borrow::Borrow], + sanitized_txs: &[impl core::borrow::Borrow], lock_results: Vec, error_counters: &mut TransactionErrorMetrics, ) -> Vec { @@ -4472,7 +4483,7 @@ impl Bank { .map(|(sanitized_tx, (lock_result, nonce, lamports))| { let sanitized_tx = sanitized_tx.borrow(); if lock_result.is_ok() - && self.is_transaction_already_processed(sanitized_tx, &rcache) + && self.is_transaction_already_processed(&sanitized_tx.transaction, &rcache) { error_counters.already_processed += 1; return (Err(TransactionError::AlreadyProcessed), None, None); @@ -4525,7 +4536,7 @@ impl Bank { pub fn check_transactions( &self, - sanitized_txs: &[impl core::borrow::Borrow], + sanitized_txs: &[impl core::borrow::Borrow], lock_results: &[Result<()>], max_age: usize, error_counters: &mut TransactionErrorMetrics, @@ -4538,7 +4549,7 @@ impl Bank { let mut balances: TransactionBalances = vec![]; for transaction in batch.sanitized_transactions() { let mut transaction_balances: Vec = vec![]; - for account_key in transaction.message().account_keys().iter() { + for account_key in transaction.transaction.message().account_keys().iter() { transaction_balances.push(self.get_balance(account_key)); } balances.push(transaction_balances); @@ -4635,7 +4646,7 @@ impl Bank { let mut collect_logs_time = Measure::start("collect_logs_time"); for (execution_result, tx) in sanitized_output.execution_results.iter().zip(sanitized_txs) { if let Some(debug_keys) = &self.transaction_debug_keys { - for key in tx.message().account_keys().iter() { + for key in tx.transaction.message().account_keys().iter() { if debug_keys.contains(key) { let result = execution_result.flattened_result(); info!("slot: {} result: {:?} tx: {:?}", self.slot, result, tx); @@ -4644,7 +4655,7 @@ impl Bank { } } - let is_vote = tx.is_simple_vote_transaction(); + let is_vote = tx.transaction.is_simple_vote_transaction(); if execution_result.was_executed() // Skip log collection for unprocessed transactions && transaction_log_collector_config.filter != TransactionLogCollectorFilter::None @@ -4654,7 +4665,7 @@ impl Bank { .mentioned_addresses .is_empty() { - for key in tx.message().account_keys().iter() { + for key in tx.transaction.message().account_keys().iter() { if transaction_log_collector_config .mentioned_addresses .contains(key) @@ -4687,7 +4698,7 @@ impl Bank { let transaction_log_index = transaction_log_collector.logs.len(); transaction_log_collector.logs.push(TransactionLogInfo { - signature: *tx.signature(), + signature: *tx.transaction.signature(), result: status.clone(), is_vote, log_messages: log_messages.clone(), @@ -4707,7 +4718,8 @@ impl Bank { // Signature count must be accumulated only if the transaction // is executed, otherwise a mismatched count between banking and // replay could occur - signature_count += u64::from(tx.message().header().num_required_signatures); + signature_count += + u64::from(tx.transaction.message().header().num_required_signatures); executed_transactions_count += 1; } @@ -4817,7 +4829,7 @@ impl Bank { fn filter_program_errors_and_collect_fee( &self, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], execution_results: &[TransactionExecutionResult], ) -> Vec> { let hash_queue = self.blockhash_queue.read().unwrap(); @@ -4839,7 +4851,9 @@ impl Bank { .map(|maybe_lamports_per_signature| (maybe_lamports_per_signature, true)) .unwrap_or_else(|| { ( - hash_queue.get_lamports_per_signature(tx.message().recent_blockhash()), + hash_queue.get_lamports_per_signature( + tx.transaction.message().recent_blockhash(), + ), false, ) }); @@ -4847,7 +4861,7 @@ impl Bank { let lamports_per_signature = lamports_per_signature.ok_or(TransactionError::BlockhashNotFound)?; let fee = self.get_fee_for_message_with_lamports_per_signature( - tx.message(), + tx.transaction.message(), lamports_per_signature, ); @@ -4859,7 +4873,7 @@ impl Bank { // post-load, fee deducted, pre-execute account state // stored if execution_status.is_err() && !is_nonce { - self.withdraw(tx.message().fee_payer(), fee)?; + self.withdraw(tx.transaction.message().fee_payer(), fee)?; } fees += fee; @@ -4877,7 +4891,7 @@ impl Bank { /// a failure result. pub fn commit_transactions( &self, - sanitized_txs: &[SanitizedTransaction], + sanitized_txs: &[ExtendedSanitizedTransaction], loaded_txs: &mut [TransactionLoadResult], execution_results: Vec, last_blockhash: Hash, @@ -6542,7 +6556,7 @@ impl Bank { &self, tx: VersionedTransaction, verification_mode: TransactionVerificationMode, - ) -> Result { + ) -> Result { let sanitized_tx = { let size = bincode::serialized_size(&tx).map_err(|_| TransactionError::SanitizeFailure)?; @@ -6565,13 +6579,13 @@ impl Bank { sanitized_tx.verify_precompiles(&self.feature_set)?; } - Ok(sanitized_tx) + Ok(ExtendedSanitizedTransaction::from(sanitized_tx)) } pub fn fully_verify_transaction( &self, tx: VersionedTransaction, - ) -> Result { + ) -> Result { self.verify_transaction(tx, TransactionVerificationMode::FullVerification) } @@ -6913,7 +6927,7 @@ impl Bank { /// a bank-level cache of vote accounts and stake delegation info fn update_stakes_cache( &self, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], execution_results: &[TransactionExecutionResult], loaded_txs: &[TransactionLoadResult], ) { @@ -6924,7 +6938,7 @@ impl Bank { .filter(|(_, execution_result, _)| execution_result.was_executed_successfully()) .flat_map(|(tx, _, (load_result, _))| { load_result.iter().flat_map(|loaded_transaction| { - let num_account_keys = tx.message().account_keys().len(); + let num_account_keys = tx.transaction.message().account_keys().len(); loaded_transaction.accounts.iter().take(num_account_keys) }) }) @@ -7446,7 +7460,7 @@ impl Bank { /// Checks a batch of sanitized transactions again bank for age and status pub fn check_transactions_with_forwarding_delay( &self, - transactions: &[SanitizedTransaction], + transactions: &[ExtendedSanitizedTransaction], filter: &[transaction::Result<()>], forward_transactions_to_leader_at_slot_offset: u64, ) -> Vec { @@ -7669,7 +7683,10 @@ impl Bank { let transaction_account_lock_limit = self.get_transaction_account_lock_limit(); let sanitized_txs = txs .into_iter() - .map(SanitizedTransaction::from_transaction_for_tests) + .map(|txn| ExtendedSanitizedTransaction { + transaction: SanitizedTransaction::from_transaction_for_tests(txn), + start_time: None, + }) .collect::>(); let lock_results = self .rc diff --git a/runtime/src/bank_utils.rs b/runtime/src/bank_utils.rs index 10835afb82dc49..645630a89a938e 100644 --- a/runtime/src/bank_utils.rs +++ b/runtime/src/bank_utils.rs @@ -7,7 +7,7 @@ use { solana_sdk::{pubkey::Pubkey, signature::Signer}, }; use { - solana_sdk::transaction::SanitizedTransaction, + solana_sdk::transaction::ExtendedSanitizedTransaction, solana_svm::transaction_results::TransactionResults, solana_vote::{vote_parser, vote_sender_types::ReplayVoteSender}, }; @@ -37,7 +37,7 @@ pub fn setup_bank_and_vote_pubkeys_for_tests( } pub fn find_and_send_votes( - sanitized_txs: &[SanitizedTransaction], + sanitized_txs: &[ExtendedSanitizedTransaction], tx_results: &TransactionResults, vote_sender: Option<&ReplayVoteSender>, ) { @@ -49,8 +49,11 @@ pub fn find_and_send_votes( .iter() .zip(execution_results.iter()) .for_each(|(tx, result)| { - if tx.is_simple_vote_transaction() && result.was_executed_successfully() { - if let Some(parsed_vote) = vote_parser::parse_sanitized_vote_transaction(tx) { + if tx.transaction.is_simple_vote_transaction() && result.was_executed_successfully() + { + if let Some(parsed_vote) = + vote_parser::parse_sanitized_vote_transaction(&tx.transaction) + { if parsed_vote.1.last_voted_slot().is_some() { let _ = vote_sender.send(parsed_vote); } diff --git a/runtime/src/installed_scheduler_pool.rs b/runtime/src/installed_scheduler_pool.rs index d39a18d567232a..ff62b69a8d87d9 100644 --- a/runtime/src/installed_scheduler_pool.rs +++ b/runtime/src/installed_scheduler_pool.rs @@ -27,7 +27,7 @@ use { solana_sdk::{ hash::Hash, slot_history::Slot, - transaction::{Result, SanitizedTransaction}, + transaction::{ExtendedSanitizedTransaction, Result}, }, std::{ fmt::Debug, @@ -104,7 +104,7 @@ pub trait InstalledScheduler: Send + Sync + Debug + 'static { // Calling this is illegal as soon as wait_for_termination is called. fn schedule_execution<'a>( &'a self, - transaction_with_index: &'a (&'a SanitizedTransaction, usize), + transaction_with_index: &'a (&'a ExtendedSanitizedTransaction, usize), ); /// Wait for a scheduler to terminate after processing. @@ -289,7 +289,9 @@ impl BankWithScheduler { // 'a is needed; anonymous_lifetime_in_impl_trait isn't stabilized yet... pub fn schedule_transaction_executions<'a>( &self, - transactions_with_indexes: impl ExactSizeIterator, + transactions_with_indexes: impl ExactSizeIterator< + Item = (&'a ExtendedSanitizedTransaction, &'a usize), + >, ) { trace!( "schedule_transaction_executions(): {} txs", diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index 0490f594451b9c..363379178174bd 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -8,7 +8,7 @@ use { solana_sdk::{ clock::{BankId, Slot}, pubkey::Pubkey, - transaction::SanitizedTransaction, + transaction::ExtendedSanitizedTransaction, }, std::{ collections::HashMap, @@ -208,20 +208,29 @@ impl PrioritizationFeeCache { /// Update with a list of non-vote transactions' compute_budget_details and account_locks; Only /// transactions have both valid compute_budget_details and account_locks will be used to update /// fee_cache asynchronously. - pub fn update<'a>(&self, bank: &Bank, txs: impl Iterator) { + pub fn update<'a>( + &self, + bank: &Bank, + txs: impl Iterator, + ) { let (_, send_updates_time) = measure!( { for sanitized_transaction in txs { // Vote transactions are not prioritized, therefore they are excluded from // updating fee_cache. - if sanitized_transaction.is_simple_vote_transaction() { + if sanitized_transaction + .transaction + .is_simple_vote_transaction() + { continue; } let round_compute_unit_price_enabled = false; // TODO: bank.feture_set.is_active(round_compute_unit_price) let compute_budget_details = sanitized_transaction + .transaction .get_compute_budget_details(round_compute_unit_price_enabled); let account_locks = sanitized_transaction + .transaction .get_account_locks(bank.get_transaction_account_lock_limit()); if compute_budget_details.is_none() || account_locks.is_err() { diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index ecec27e02e93aa..fe1c8bc3210ea9 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -1,6 +1,6 @@ use { crate::bank::Bank, - solana_sdk::transaction::{Result, SanitizedTransaction}, + solana_sdk::transaction::{ExtendedSanitizedTransaction, Result}, std::borrow::Cow, }; @@ -8,7 +8,7 @@ use { pub struct TransactionBatch<'a, 'b> { lock_results: Vec>, bank: &'a Bank, - sanitized_txs: Cow<'b, [SanitizedTransaction]>, + sanitized_txs: Cow<'b, [ExtendedSanitizedTransaction]>, needs_unlock: bool, } @@ -16,7 +16,7 @@ impl<'a, 'b> TransactionBatch<'a, 'b> { pub fn new( lock_results: Vec>, bank: &'a Bank, - sanitized_txs: Cow<'b, [SanitizedTransaction]>, + sanitized_txs: Cow<'b, [ExtendedSanitizedTransaction]>, ) -> Self { assert_eq!(lock_results.len(), sanitized_txs.len()); Self { @@ -31,7 +31,7 @@ impl<'a, 'b> TransactionBatch<'a, 'b> { &self.lock_results } - pub fn sanitized_transactions(&self) -> &[SanitizedTransaction] { + pub fn sanitized_transactions(&self) -> &[ExtendedSanitizedTransaction] { &self.sanitized_txs } diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index b7383b4a0a454c..2d4e2bd21018dc 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -19,6 +19,7 @@ use { transaction::{Result, Transaction, TransactionError, VersionedTransaction}, }, solana_program::message::SanitizedVersionedMessage, + std::time::Instant, }; /// Maximum number of accounts that a transaction may lock. @@ -35,6 +36,22 @@ pub struct SanitizedTransaction { signatures: Vec, } +/// Sanitized transaction with option start_time +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct ExtendedSanitizedTransaction { + pub transaction: SanitizedTransaction, + pub start_time: Option, +} + +impl From for ExtendedSanitizedTransaction { + fn from(value: SanitizedTransaction) -> Self { + Self { + transaction: value, + start_time: None, + } + } +} + /// Set of accounts that must be locked for safe transaction processing #[derive(Debug, Clone, Default, Eq, PartialEq)] pub struct TransactionAccountLocks<'a> { diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index bf9b5b9c40bfee..5b9b2a1f8f0835 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -30,7 +30,7 @@ use { rent_debits::RentDebits, saturating_add_assign, sysvar::{self, instructions::construct_instructions_data}, - transaction::{self, Result, SanitizedTransaction, TransactionError}, + transaction::{self, ExtendedSanitizedTransaction, Result, TransactionError}, transaction_context::{IndexOfAccount, TransactionAccount}, }, solana_system_program::{get_system_account_kind, SystemAccountKind}, @@ -112,7 +112,7 @@ pub fn validate_fee_payer( /// second element. pub(crate) fn load_accounts( callbacks: &CB, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], lock_results: &[TransactionCheckResult], error_counters: &mut TransactionErrorMetrics, fee_structure: &FeeStructure, @@ -125,7 +125,7 @@ pub(crate) fn load_accounts( .zip(lock_results) .map(|etx| match etx { (tx, (Ok(()), nonce, lamports_per_signature)) => { - let message = tx.message(); + let message = tx.transaction.message(); let fee = if let Some(lamports_per_signature) = lamports_per_signature { fee_structure.calculate_fee( message, diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 5801b3b8316fdc..98b930c6b8b3c0 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -43,7 +43,7 @@ use { pubkey::Pubkey, rent_collector::RentCollector, saturating_add_assign, - transaction::{self, SanitizedTransaction, TransactionError}, + transaction::{self, ExtendedSanitizedTransaction, SanitizedTransaction, TransactionError}, transaction_context::{ExecutionRecord, TransactionContext}, }, std::{ @@ -200,7 +200,7 @@ impl TransactionBatchProcessor { pub fn load_and_execute_sanitized_transactions<'a, CB: TransactionProcessingCallback>( &self, callbacks: &CB, - sanitized_txs: &[SanitizedTransaction], + sanitized_txs: &[ExtendedSanitizedTransaction], check_results: &mut [TransactionCheckResult], error_counters: &mut TransactionErrorMetrics, recording_config: ExecutionRecordingConfig, @@ -262,7 +262,7 @@ impl TransactionBatchProcessor { let mut compute_budget_process_transaction_time = Measure::start("compute_budget_process_transaction_time"); let maybe_compute_budget = ComputeBudget::try_from_instructions( - tx.message().program_instructions_iter(), + tx.transaction.message().program_instructions_iter(), ); compute_budget_process_transaction_time.stop(); saturating_add_assign!( @@ -279,7 +279,7 @@ impl TransactionBatchProcessor { let result = self.execute_loaded_transaction( callbacks, - tx, + &tx.transaction, loaded_transaction, compute_budget, nonce.as_ref().map(DurableNonceFee::from), @@ -341,7 +341,7 @@ impl TransactionBatchProcessor { /// blockhash or nonce. pub fn filter_executable_program_accounts<'a, CB: TransactionProcessingCallback>( callbacks: &CB, - txs: &[SanitizedTransaction], + txs: &[ExtendedSanitizedTransaction], lock_results: &mut [TransactionCheckResult], program_owners: &'a [Pubkey], ) -> HashMap { @@ -349,7 +349,8 @@ impl TransactionBatchProcessor { lock_results.iter_mut().zip(txs).for_each(|etx| { if let ((Ok(()), _nonce, lamports_per_signature), tx) = etx { if lamports_per_signature.is_some() { - tx.message() + tx.transaction + .message() .account_keys() .iter() .for_each(|key| match result.entry(*key) { diff --git a/unified-scheduler-logic/src/lib.rs b/unified-scheduler-logic/src/lib.rs index 997c6c1745a7c9..e587c3ba06f38d 100644 --- a/unified-scheduler-logic/src/lib.rs +++ b/unified-scheduler-logic/src/lib.rs @@ -1,12 +1,12 @@ -use solana_sdk::transaction::SanitizedTransaction; +use solana_sdk::transaction::ExtendedSanitizedTransaction; pub struct Task { - transaction: SanitizedTransaction, + transaction: ExtendedSanitizedTransaction, index: usize, } impl Task { - pub fn create_task(transaction: SanitizedTransaction, index: usize) -> Self { + pub fn create_task(transaction: ExtendedSanitizedTransaction, index: usize) -> Self { Task { transaction, index } } @@ -14,7 +14,7 @@ impl Task { self.index } - pub fn transaction(&self) -> &SanitizedTransaction { + pub fn transaction(&self) -> &ExtendedSanitizedTransaction { &self.transaction } } diff --git a/unified-scheduler-pool/src/lib.rs b/unified-scheduler-pool/src/lib.rs index 09ded82ee88e7d..4d8780c278a3c9 100644 --- a/unified-scheduler-pool/src/lib.rs +++ b/unified-scheduler-pool/src/lib.rs @@ -26,7 +26,7 @@ use { }, prioritization_fee_cache::PrioritizationFeeCache, }, - solana_sdk::transaction::{Result, SanitizedTransaction}, + solana_sdk::transaction::{ExtendedSanitizedTransaction, Result}, solana_unified_scheduler_logic::Task, solana_vote::vote_sender_types::ReplayVoteSender, std::{ @@ -203,7 +203,7 @@ pub trait TaskHandler: Send + Sync + Debug + Sized + 'static { result: &mut Result<()>, timings: &mut ExecuteTimings, bank: &Arc, - transaction: &SanitizedTransaction, + transaction: &ExtendedSanitizedTransaction, index: usize, handler_context: &HandlerContext, ); @@ -217,7 +217,7 @@ impl TaskHandler for DefaultTaskHandler { result: &mut Result<()>, timings: &mut ExecuteTimings, bank: &Arc, - transaction: &SanitizedTransaction, + transaction: &ExtendedSanitizedTransaction, index: usize, handler_context: &HandlerContext, ) { @@ -740,7 +740,7 @@ impl InstalledScheduler for PooledScheduler { &self.context } - fn schedule_execution(&self, &(transaction, index): &(&SanitizedTransaction, usize)) { + fn schedule_execution(&self, &(transaction, index): &(&ExtendedSanitizedTransaction, usize)) { let task = Task::create_task(transaction.clone(), index); self.inner.thread_manager.send_task(task); } From b261b2534ee51792a602cea8f5fc3b1f275bc348 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Sat, 2 Mar 2024 03:22:27 -0800 Subject: [PATCH 17/42] Fixed a clippy issue --- runtime/src/bank.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 393291b6ea18fd..94f58d4410a4cd 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4204,14 +4204,12 @@ impl Bank { let sanitized_txs = txs .into_iter() .map(|tx| { - SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self).and_then( - |txn| { - Ok(ExtendedSanitizedTransaction { - transaction: txn, - start_time: None, - }) - }, - ) + SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self).map(|txn| { + ExtendedSanitizedTransaction { + transaction: txn, + start_time: None, + } + }) }) .collect::>>()?; let tx_account_lock_limit = self.get_transaction_account_lock_limit(); From 79a0f225086d57708c8069685f204b8368bed6c0 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Sun, 3 Mar 2024 02:53:26 -0800 Subject: [PATCH 18/42] instrument to pass the perf_track_metrics down the transaction processing stack --- Cargo.lock | 2 + core/src/banking_stage/consume_worker.rs | 17 ++++-- core/src/banking_stage/consumer.rs | 55 ++++++++----------- core/src/banking_stage/leader_slot_metrics.rs | 20 ++++--- ledger/src/blockstore_processor.rs | 1 + runtime/Cargo.toml | 1 + runtime/src/bank.rs | 8 +++ svm/Cargo.toml | 1 + svm/src/transaction_processor.rs | 12 ++++ 9 files changed, 72 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3459ace3b811a6..62c4f2542e3629 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6994,6 +6994,7 @@ dependencies = [ "ed25519-dalek", "flate2", "fnv", + "histogram", "im", "index_list", "itertools", @@ -7292,6 +7293,7 @@ name = "solana-svm" version = "2.0.0" dependencies = [ "bincode", + "histogram", "itertools", "log", "percentage", diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 92fb07ddfab18c..779e9db4e4ea02 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -34,6 +34,7 @@ pub(crate) struct ConsumeWorker { leader_bank_notifier: Arc, metrics: Arc, + perf_track_metrics: histogram::Histogram, } #[allow(dead_code)] @@ -51,6 +52,7 @@ impl ConsumeWorker { consumed_sender, leader_bank_notifier, metrics: Arc::new(ConsumeWorkerMetrics::new(id)), + perf_track_metrics: histogram::Histogram::default(), } } @@ -58,19 +60,23 @@ impl ConsumeWorker { self.metrics.clone() } - pub fn run(self) -> Result<(), ConsumeWorkerError> { + pub fn run(mut self) -> Result<(), ConsumeWorkerError> { loop { let work = self.consume_receiver.recv()?; self.consume_loop(work)?; } } - fn consume_loop(&self, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { + fn consume_loop(&mut self, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { let Some(mut bank) = self.get_consume_bank() else { return self.retry_drain(work); }; - for work in try_drain_iter(work, &self.consume_receiver) { + // We need to have a mutable borrow in consume, so move the receiver out + let (_, dummy_receiver) = crossbeam_channel::unbounded(); + let consume_receiver = std::mem::replace(&mut self.consume_receiver, dummy_receiver); + + for work in try_drain_iter(work, &consume_receiver) { if bank.is_complete() { if let Some(new_bank) = self.get_consume_bank() { bank = new_bank; @@ -81,15 +87,18 @@ impl ConsumeWorker { self.consume(&bank, work)?; } + // restore it + self.consume_receiver = consume_receiver; Ok(()) } /// Consume a single batch. - fn consume(&self, bank: &Arc, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { + fn consume(&mut self, bank: &Arc, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { let output = self.consumer.process_and_record_aged_transactions( bank, &work.transactions, &work.max_age_slots, + Some(&mut self.perf_track_metrics), ); self.metrics.update_for_consume(&output); diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 0c5ae73c5ba651..b9b80f01bb0dc3 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -208,33 +208,6 @@ impl Consumer { payload .slot_metrics_tracker .increment_retryable_packets_count(retryable_transaction_indexes.len() as u64); - - // Now we track the performance for the interested transactions which is not in the retryable_transaction_indexes - // We assume the retryable_transaction_indexes is already sorted. - let mut retryable_idx = 0; - for (index, packet) in packets_to_process.iter().enumerate() { - if packet.original_packet().meta().is_perf_track_packet() { - if let Some(start_time) = packet.start_time() { - if retryable_idx >= retryable_transaction_indexes.len() - || retryable_transaction_indexes[retryable_idx] != index - { - let duration = Instant::now().duration_since(*start_time); - - debug!( - "Banking stage processing took {duration:?} for transaction {:?}", - packet.transaction().get_signatures().first() - ); - payload - .slot_metrics_tracker - .increment_process_sampled_packets_us(duration.as_micros() as u64); - } else { - // This packet is retried, advance the retry index to the next, as the next packet's index will - // certainly be > than this. - retryable_idx += 1; - } - } - } - } Some(retryable_transaction_indexes) } @@ -246,9 +219,13 @@ impl Consumer { banking_stage_stats: &BankingStageStats, slot_metrics_tracker: &mut LeaderSlotMetricsTracker, ) -> ProcessTransactionsSummary { - let (mut process_transactions_summary, process_transactions_us) = measure_us!( - self.process_transactions(bank, bank_creation_time, sanitized_transactions) - ); + let (mut process_transactions_summary, process_transactions_us) = measure_us!(self + .process_transactions( + bank, + bank_creation_time, + sanitized_transactions, + &mut slot_metrics_tracker.get_transaction_perf_track_metrics_ref() + )); slot_metrics_tracker.increment_process_transactions_us(process_transactions_us); banking_stage_stats .transaction_processing_elapsed @@ -299,6 +276,7 @@ impl Consumer { bank: &Arc, bank_creation_time: &Instant, transactions: &[ExtendedSanitizedTransaction], + perf_track_metrics: &mut Option<&mut histogram::Histogram>, ) -> ProcessTransactionsSummary { let mut chunk_start = 0; let mut all_retryable_tx_indexes = vec![]; @@ -328,6 +306,7 @@ impl Consumer { bank, &transactions[chunk_start..chunk_end], chunk_start, + perf_track_metrics, ); let ProcessTransactionBatchOutput { @@ -434,6 +413,7 @@ impl Consumer { bank: &Arc, txs: &[ExtendedSanitizedTransaction], chunk_offset: usize, + perf_track_metrics: &mut Option<&mut histogram::Histogram>, ) -> ProcessTransactionBatchOutput { let mut error_counters = TransactionErrorMetrics::default(); let pre_results = vec![Ok(()); txs.len()]; @@ -447,6 +427,7 @@ impl Consumer { txs, chunk_offset, check_results, + perf_track_metrics, ); // Accumulate error counters from the initial checks into final results @@ -462,6 +443,7 @@ impl Consumer { bank: &Arc, txs: &[ExtendedSanitizedTransaction], max_slot_ages: &[Slot], + mut perf_track_metrics: Option<&mut histogram::Histogram>, ) -> ProcessTransactionBatchOutput { // Need to filter out transactions since they were sanitized earlier. // This means that the transaction may cross and epoch boundary (not allowed), @@ -487,7 +469,13 @@ impl Consumer { } Ok(()) }); - self.process_and_record_transactions_with_pre_results(bank, txs, 0, pre_results) + self.process_and_record_transactions_with_pre_results( + bank, + txs, + 0, + pre_results, + &mut perf_track_metrics, + ) } fn process_and_record_transactions_with_pre_results( @@ -496,6 +484,7 @@ impl Consumer { txs: &[ExtendedSanitizedTransaction], chunk_offset: usize, pre_results: impl Iterator>, + perf_track_metrics: &mut Option<&mut histogram::Histogram>, ) -> ProcessTransactionBatchOutput { let ( (transaction_qos_cost_results, cost_model_throttled_transactions_count), @@ -521,7 +510,7 @@ impl Consumer { // WouldExceedMaxAccountCostLimit, WouldExceedMaxVoteCostLimit // and WouldExceedMaxAccountDataCostLimit let mut execute_and_commit_transactions_output = - self.execute_and_commit_transactions_locked(bank, &batch); + self.execute_and_commit_transactions_locked(bank, &batch, perf_track_metrics); // Once the accounts are new transactions can enter the pipeline to process them let (_, unlock_us) = measure_us!(drop(batch)); @@ -587,6 +576,7 @@ impl Consumer { &self, bank: &Arc, batch: &TransactionBatch, + perf_track_metrics: &mut Option<&mut histogram::Histogram>, ) -> ExecuteAndCommitTransactionsOutput { let transaction_status_sender_enabled = self.committer.transaction_status_sender_enabled(); let mut execute_and_commit_timings = LeaderExecuteAndCommitTimings::default(); @@ -626,6 +616,7 @@ impl Consumer { None, // account_overrides self.log_messages_bytes_limit, true, + perf_track_metrics, )); execute_and_commit_timings.load_execute_us = load_execute_us; diff --git a/core/src/banking_stage/leader_slot_metrics.rs b/core/src/banking_stage/leader_slot_metrics.rs index 1c255ca019bfe7..301df48a4f2582 100644 --- a/core/src/banking_stage/leader_slot_metrics.rs +++ b/core/src/banking_stage/leader_slot_metrics.rs @@ -937,15 +937,17 @@ impl LeaderSlotMetricsTracker { } } - pub(crate) fn increment_process_sampled_packets_us(&mut self, us: u64) { - if let Some(leader_slot_metrics) = &mut self.leader_slot_metrics { - leader_slot_metrics - .timing_metrics - .process_packets_timings - .process_sampled_packets_us_hist - .increment(us) - .unwrap(); - } + pub(crate) fn get_transaction_perf_track_metrics_ref( + &mut self, + ) -> Option<&mut histogram::Histogram> { + self.leader_slot_metrics + .as_mut() + .map(|leader_slot_metrics| { + &mut leader_slot_metrics + .timing_metrics + .process_packets_timings + .process_sampled_packets_us_hist + }) } } diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index d478c8fa6dfa1b..64f37f9ad80fbf 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -169,6 +169,7 @@ pub fn execute_batch( ExecutionRecordingConfig::new_single_setting(transaction_status_sender.is_some()), timings, log_messages_bytes_limit, + None, ); bank_utils::find_and_send_votes( diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 02553d4215909d..85600e9de319da 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -24,6 +24,7 @@ dashmap = { workspace = true, features = ["rayon", "raw-api"] } dir-diff = { workspace = true } flate2 = { workspace = true } fnv = { workspace = true } +histogram = { workspace = true } im = { workspace = true, features = ["rayon", "serde"] } index_list = { workspace = true } itertools = { workspace = true } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 94f58d4410a4cd..613257cb6c9f11 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4316,6 +4316,7 @@ impl Bank { Some(&account_overrides), None, true, + &mut None, ); let post_simulation_accounts = loaded_transactions @@ -4565,6 +4566,7 @@ impl Bank { account_overrides: Option<&AccountOverrides>, log_messages_bytes_limit: Option, limit_to_load_programs: bool, + perf_track_metrics: &mut Option<&mut histogram::Histogram>, ) -> LoadAndExecuteTransactionsOutput { let sanitized_txs = batch.sanitized_transactions(); debug!("processing transactions: {}", sanitized_txs.len()); @@ -4625,6 +4627,7 @@ impl Bank { &mut check_results, &mut error_counters, recording_config, + perf_track_metrics, timings, account_overrides, self.builtin_programs.iter(), @@ -5648,6 +5651,7 @@ impl Bank { /// Process a batch of transactions. #[must_use] + #[allow(clippy::too_many_arguments)] pub fn load_execute_and_commit_transactions( &self, batch: &TransactionBatch, @@ -5656,6 +5660,7 @@ impl Bank { recording_config: ExecutionRecordingConfig, timings: &mut ExecuteTimings, log_messages_bytes_limit: Option, + mut perf_track_metrics: Option<&mut histogram::Histogram>, ) -> (TransactionResults, TransactionBalancesSet) { let pre_balances = if collect_balances { self.collect_balances(batch) @@ -5679,6 +5684,7 @@ impl Bank { None, log_messages_bytes_limit, false, + &mut perf_track_metrics, ); let (last_blockhash, lamports_per_signature) = @@ -5749,6 +5755,7 @@ impl Bank { }, &mut ExecuteTimings::default(), Some(1000 * 1000), + None, ); execution_results.remove(0) @@ -5785,6 +5792,7 @@ impl Bank { ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, + None, ) .0 .fee_collection_results diff --git a/svm/Cargo.toml b/svm/Cargo.toml index 21da2f7105bd73..ca4fba3a6e2b06 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -11,6 +11,7 @@ edition = { workspace = true } [dependencies] itertools = { workspace = true } +histogram = { workspace = true } log = { workspace = true } percentage = { workspace = true } solana-bpf-loader-program = { workspace = true } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 98b930c6b8b3c0..e3b5de22159a1d 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -43,6 +43,7 @@ use { pubkey::Pubkey, rent_collector::RentCollector, saturating_add_assign, + timing::duration_as_us, transaction::{self, ExtendedSanitizedTransaction, SanitizedTransaction, TransactionError}, transaction_context::{ExecutionRecord, TransactionContext}, }, @@ -52,6 +53,7 @@ use { fmt::{Debug, Formatter}, rc::Rc, sync::{atomic::Ordering, Arc, RwLock}, + time::Instant, }, }; @@ -204,6 +206,7 @@ impl TransactionBatchProcessor { check_results: &mut [TransactionCheckResult], error_counters: &mut TransactionErrorMetrics, recording_config: ExecutionRecordingConfig, + perf_track_metrics: &mut Option<&mut histogram::Histogram>, timings: &mut ExecuteTimings, account_overrides: Option<&AccountOverrides>, builtin_programs: impl Iterator, @@ -295,6 +298,15 @@ impl TransactionBatchProcessor { programs_modified_by_tx, } = &result { + if let Some(perf_track_metrics) = perf_track_metrics.as_mut() { + if let Some(start_time) = &tx.start_time { + // measure the time from start of banking stage to the execution of the transaction + let duration = Instant::now().duration_since(*start_time); + perf_track_metrics + .increment(duration_as_us(&duration)) + .unwrap(); + } + } // Update batch specific cache of the loaded programs with the modifications // made by the transaction, if it executed successfully. if details.status.is_ok() { From 956372bc7ed63ee0d899a464ee43601282d0c9e6 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Sun, 10 Mar 2024 22:08:28 -0700 Subject: [PATCH 19/42] move ExtendedSanitizedTransaction to solana-runtime-transaction crate --- Cargo.lock | 10 ++++++++++ accounts-db/Cargo.toml | 1 + accounts-db/src/accounts.rs | 5 ++--- accounts-db/src/accounts_db.rs | 2 +- banks-server/Cargo.toml | 1 + banks-server/src/banks_server.rs | 6 ++---- core/Cargo.toml | 1 + core/src/banking_stage/consumer.rs | 3 ++- .../forward_packet_batches_by_accounts.rs | 3 ++- .../banking_stage/latest_unprocessed_votes.rs | 2 +- core/src/banking_stage/qos_service.rs | 3 ++- core/src/banking_stage/scheduler_messages.rs | 3 ++- .../prio_graph_scheduler.rs | 6 ++---- .../scheduler_controller.rs | 3 ++- .../transaction_scheduler/transaction_state.rs | 5 ++++- .../unprocessed_transaction_storage.rs | 8 +++----- entry/Cargo.toml | 1 + entry/src/entry.rs | 5 +++-- ledger/Cargo.toml | 1 + ledger/src/blockstore_processor.rs | 4 ++-- rpc/Cargo.toml | 1 + rpc/src/rpc.rs | 6 ++---- runtime-transaction/src/lib.rs | 1 + runtime/Cargo.toml | 1 + runtime/src/bank.rs | 6 +++--- runtime/src/bank_utils.rs | 2 +- runtime/src/installed_scheduler_pool.rs | 7 ++----- runtime/src/prioritization_fee_cache.rs | 2 +- runtime/src/transaction_batch.rs | 4 ++-- sdk/src/transaction/sanitized.rs | 17 ----------------- svm/Cargo.toml | 1 + svm/src/account_loader.rs | 3 ++- svm/src/transaction_processor.rs | 3 ++- unified-scheduler-logic/Cargo.toml | 1 + unified-scheduler-logic/src/lib.rs | 2 +- unified-scheduler-pool/Cargo.toml | 1 + unified-scheduler-pool/src/lib.rs | 3 ++- 37 files changed, 69 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62c4f2542e3629..623e90fe3917a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5432,6 +5432,7 @@ dependencies = [ "solana-nohash-hasher", "solana-program-runtime", "solana-rayon-threadlimit", + "solana-runtime-transaction", "solana-sdk", "solana-stake-program", "solana-svm", @@ -5536,6 +5537,7 @@ dependencies = [ "solana-banks-interface", "solana-client", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-send-transaction-service", "solana-svm", @@ -6005,6 +6007,7 @@ dependencies = [ "solana-rpc", "solana-rpc-client-api", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-send-transaction-service", "solana-stake-program", @@ -6129,6 +6132,7 @@ dependencies = [ "solana-metrics", "solana-perf", "solana-rayon-threadlimit", + "solana-runtime-transaction", "solana-sdk", ] @@ -6365,6 +6369,7 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-stake-program", "solana-storage-bigtable", @@ -6860,6 +6865,7 @@ dependencies = [ "solana-rayon-threadlimit", "solana-rpc-client-api", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-send-transaction-service", "solana-stake-program", @@ -7038,6 +7044,7 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-stake-program", "solana-svm", @@ -7306,6 +7313,7 @@ dependencies = [ "solana-measure", "solana-metrics", "solana-program-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-system-program", ] @@ -7541,6 +7549,7 @@ dependencies = [ name = "solana-unified-scheduler-logic" version = "2.0.0" dependencies = [ + "solana-runtime-transaction", "solana-sdk", ] @@ -7556,6 +7565,7 @@ dependencies = [ "solana-logger", "solana-program-runtime", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-unified-scheduler-logic", "solana-vote", diff --git a/accounts-db/Cargo.toml b/accounts-db/Cargo.toml index 0fc5a381fbda5e..b575e47db07299 100644 --- a/accounts-db/Cargo.toml +++ b/accounts-db/Cargo.toml @@ -52,6 +52,7 @@ solana-metrics = { workspace = true } solana-nohash-hasher = { workspace = true } solana-program-runtime = { workspace = true } solana-rayon-threadlimit = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-stake-program = { workspace = true } solana-svm = { workspace = true } diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 5702353f9fc142..b72b47f715394c 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -10,6 +10,7 @@ use { }, dashmap::DashMap, log::*, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::{AccountSharedData, ReadableAccount}, account_utils::StateMut, @@ -23,9 +24,7 @@ use { nonce_info::{NonceFull, NonceInfo}, pubkey::Pubkey, slot_hashes::SlotHashes, - transaction::{ - ExtendedSanitizedTransaction, Result, TransactionAccountLocks, TransactionError, - }, + transaction::{Result, TransactionAccountLocks, TransactionError}, transaction_context::TransactionAccount, }, solana_svm::{ diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 648be7fd5b167b..5a0a6ea32c95ac 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -83,6 +83,7 @@ use { solana_measure::{measure::Measure, measure_us}, solana_nohash_hasher::{IntMap, IntSet}, solana_rayon_threadlimit::get_thread_count, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, clock::{BankId, Epoch, Slot}, @@ -93,7 +94,6 @@ use { rent_collector::RentCollector, saturating_add_assign, timing::AtomicInterval, - transaction::ExtendedSanitizedTransaction, }, std::{ borrow::{Borrow, Cow}, diff --git a/banks-server/Cargo.toml b/banks-server/Cargo.toml index 6cf5f77f92548b..357a07bddd44b3 100644 --- a/banks-server/Cargo.toml +++ b/banks-server/Cargo.toml @@ -16,6 +16,7 @@ futures = { workspace = true } solana-banks-interface = { workspace = true } solana-client = { workspace = true } solana-runtime = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-send-transaction-service = { workspace = true } solana-svm = { workspace = true } diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 973c591e622edb..8cca21009edc79 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -13,6 +13,7 @@ use { bank_forks::BankForks, commitment::BlockCommitmentCache, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::Account, clock::Slot, @@ -23,10 +24,7 @@ use { message::{Message, SanitizedMessage}, pubkey::Pubkey, signature::Signature, - transaction::{ - self, ExtendedSanitizedTransaction, MessageHash, SanitizedTransaction, - VersionedTransaction, - }, + transaction::{self, MessageHash, SanitizedTransaction, VersionedTransaction}, }, solana_send_transaction_service::{ send_transaction_service::{SendTransactionService, TransactionInfo}, diff --git a/core/Cargo.toml b/core/Cargo.toml index 1fd25ec38a8d3b..4308dc3fba3a43 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -62,6 +62,7 @@ solana-rayon-threadlimit = { workspace = true } solana-rpc = { workspace = true } solana-rpc-client-api = { workspace = true } solana-runtime = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-send-transaction-service = { workspace = true } solana-streamer = { workspace = true } diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index b9b80f01bb0dc3..b55019130c3ca1 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -23,13 +23,14 @@ use { compute_budget_details::GetComputeBudgetDetails, transaction_batch::TransactionBatch, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::{Slot, FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE}, feature_set, message::SanitizedMessage, saturating_add_assign, timing::timestamp, - transaction::{self, AddressLoader, ExtendedSanitizedTransaction, TransactionError}, + transaction::{self, AddressLoader, TransactionError}, }, solana_svm::{ account_loader::{validate_fee_payer, TransactionCheckResult}, diff --git a/core/src/banking_stage/forward_packet_batches_by_accounts.rs b/core/src/banking_stage/forward_packet_batches_by_accounts.rs index ab8f52820e8eac..6b1685c8a46483 100644 --- a/core/src/banking_stage/forward_packet_batches_by_accounts.rs +++ b/core/src/banking_stage/forward_packet_batches_by_accounts.rs @@ -6,7 +6,8 @@ use { cost_tracker::{CostTracker, CostTrackerError}, }, solana_perf::packet::Packet, - solana_sdk::{feature_set::FeatureSet, transaction::ExtendedSanitizedTransaction}, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::feature_set::FeatureSet, std::sync::Arc, }; diff --git a/core/src/banking_stage/latest_unprocessed_votes.rs b/core/src/banking_stage/latest_unprocessed_votes.rs index fecec111bd660c..f550c680112c98 100644 --- a/core/src/banking_stage/latest_unprocessed_votes.rs +++ b/core/src/banking_stage/latest_unprocessed_votes.rs @@ -7,11 +7,11 @@ use { rand::{thread_rng, Rng}, solana_perf::packet::Packet, solana_runtime::bank::Bank, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::{Slot, UnixTimestamp}, program_utils::limited_deserialize, pubkey::Pubkey, - transaction::ExtendedSanitizedTransaction, }, solana_vote_program::vote_instruction::VoteInstruction, std::{ diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 5532d36eb39ecd..91b89a9e619cc1 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -8,11 +8,12 @@ use { solana_cost_model::{cost_model::CostModel, transaction_cost::TransactionCost}, solana_measure::measure::Measure, solana_runtime::bank::Bank, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::Slot, feature_set::FeatureSet, saturating_add_assign, - transaction::{self, ExtendedSanitizedTransaction, TransactionError}, + transaction::{self, TransactionError}, }, std::sync::atomic::{AtomicU64, Ordering}, }; diff --git a/core/src/banking_stage/scheduler_messages.rs b/core/src/banking_stage/scheduler_messages.rs index 359f4cac710b71..b1836b4db54f22 100644 --- a/core/src/banking_stage/scheduler_messages.rs +++ b/core/src/banking_stage/scheduler_messages.rs @@ -1,6 +1,7 @@ use { super::immutable_deserialized_packet::ImmutableDeserializedPacket, - solana_sdk::{clock::Slot, transaction::ExtendedSanitizedTransaction}, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::clock::Slot, std::{fmt::Display, sync::Arc}, }; 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 95408800852b84..a80cdabb668ea6 100644 --- a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs +++ b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs @@ -16,10 +16,8 @@ use { itertools::izip, prio_graph::{AccessKind, PrioGraph}, solana_measure::measure_us, - solana_sdk::{ - pubkey::Pubkey, saturating_add_assign, slot_history::Slot, - transaction::ExtendedSanitizedTransaction, - }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::{pubkey::Pubkey, saturating_add_assign, slot_history::Slot}, }; pub(crate) struct PrioGraphScheduler { diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index 600c7ef6c3f3a3..e3aa895fddc54c 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -23,6 +23,7 @@ use { solana_measure::measure_us, solana_program_runtime::compute_budget_processor::process_compute_budget_instructions, solana_runtime::{bank::Bank, bank_forks::BankForks}, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::MAX_PROCESSING_AGE, feature_set::{ @@ -31,7 +32,7 @@ use { }, fee::FeeBudgetLimits, saturating_add_assign, - transaction::{ExtendedSanitizedTransaction, SanitizedTransaction}, + transaction::SanitizedTransaction, }, solana_svm::transaction_error_metrics::TransactionErrorMetrics, std::{ diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state.rs b/core/src/banking_stage/transaction_scheduler/transaction_state.rs index 7ae094258a9bad..22bf8a7b87344a 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state.rs @@ -1,4 +1,7 @@ -use solana_sdk::{clock::Slot, transaction::ExtendedSanitizedTransaction}; +use { + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::clock::Slot, +}; /// Simple wrapper type to tie a sanitized transaction to max age slot. pub(crate) struct SanitizedTransactionTTL { diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 7c50e045b89f62..02d61c42b894b7 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -19,12 +19,10 @@ use { min_max_heap::MinMaxHeap, solana_measure::{measure, measure_us}, solana_runtime::bank::Bank, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ - clock::FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, - feature_set::FeatureSet, - hash::Hash, - saturating_add_assign, - transaction::{ExtendedSanitizedTransaction, SanitizedTransaction}, + clock::FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, feature_set::FeatureSet, hash::Hash, + saturating_add_assign, transaction::SanitizedTransaction, }, solana_svm::transaction_error_metrics::TransactionErrorMetrics, std::{ diff --git a/entry/Cargo.toml b/entry/Cargo.toml index a9bde85d833e7c..984c08a2a93e18 100644 --- a/entry/Cargo.toml +++ b/entry/Cargo.toml @@ -23,6 +23,7 @@ solana-merkle-tree = { workspace = true } solana-metrics = { workspace = true } solana-perf = { workspace = true } solana-rayon-threadlimit = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } [dev-dependencies] diff --git a/entry/src/entry.rs b/entry/src/entry.rs index 0029148dcee18a..3ea1ca7e51551a 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -22,13 +22,14 @@ use { sigverify, }, solana_rayon_threadlimit::get_max_thread_count, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ hash::Hash, packet::Meta, timing, transaction::{ - ExtendedSanitizedTransaction, Result, Transaction, TransactionError, - TransactionVerificationMode, VersionedTransaction, + Result, Transaction, TransactionError, TransactionVerificationMode, + VersionedTransaction, }, }, std::{ diff --git a/ledger/Cargo.toml b/ledger/Cargo.toml index 7665428981ed82..8d3de706183b4e 100644 --- a/ledger/Cargo.toml +++ b/ledger/Cargo.toml @@ -49,6 +49,7 @@ solana-metrics = { workspace = true } solana-perf = { workspace = true } solana-program-runtime = { workspace = true } solana-rayon-threadlimit = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-runtime = { workspace = true } solana-sdk = { workspace = true } solana-stake-program = { workspace = true } diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 64f37f9ad80fbf..b9f85dab6a0660 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -42,6 +42,7 @@ use { prioritization_fee_cache::PrioritizationFeeCache, transaction_batch::TransactionBatch, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::{Slot, MAX_PROCESSING_AGE}, feature_set, @@ -53,8 +54,7 @@ use { signature::{Keypair, Signature}, timing, transaction::{ - ExtendedSanitizedTransaction, Result, TransactionError, TransactionVerificationMode, - VersionedTransaction, + Result, TransactionError, TransactionVerificationMode, VersionedTransaction, }, }, solana_svm::{ diff --git a/rpc/Cargo.toml b/rpc/Cargo.toml index d4f2648b6b1078..8a5c7f57c5b1ea 100644 --- a/rpc/Cargo.toml +++ b/rpc/Cargo.toml @@ -43,6 +43,7 @@ solana-poh = { workspace = true } solana-rayon-threadlimit = { workspace = true } solana-rpc-client-api = { workspace = true } solana-runtime = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-send-transaction-service = { workspace = true } solana-stake-program = { workspace = true } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index dc343a2cf05dc2..7b8a09c37da55f 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3252,10 +3252,8 @@ pub mod rpc_accounts_scan { pub mod rpc_full { use { super::*, - solana_sdk::{ - message::{SanitizedVersionedMessage, VersionedMessage}, - transaction::ExtendedSanitizedTransaction, - }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::message::{SanitizedVersionedMessage, VersionedMessage}, solana_transaction_status::UiInnerInstructions, }; #[rpc] diff --git a/runtime-transaction/src/lib.rs b/runtime-transaction/src/lib.rs index 0fdeb7c5b6bd65..4868e1f77275fc 100644 --- a/runtime-transaction/src/lib.rs +++ b/runtime-transaction/src/lib.rs @@ -1,5 +1,6 @@ #![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] #![allow(clippy::arithmetic_side_effects)] +pub mod extended_transaction; pub mod runtime_transaction; pub mod transaction_meta; diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 85600e9de319da..e439f92ef008c0 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -62,6 +62,7 @@ solana-metrics = { workspace = true } solana-perf = { workspace = true } solana-program-runtime = { workspace = true } solana-rayon-threadlimit = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-stake-program = { workspace = true } solana-svm = { workspace = true } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 613257cb6c9f11..7ee51d5304922c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -102,6 +102,7 @@ use { runtime_config::RuntimeConfig, timings::{ExecuteTimingType, ExecuteTimings}, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::{ create_account_shared_data_with_fields as create_account, create_executable_meta, @@ -152,9 +153,8 @@ use { sysvar::{self, last_restart_slot::LastRestartSlot, Sysvar, SysvarId}, timing::years_as_slots, transaction::{ - self, ExtendedSanitizedTransaction, MessageHash, Result, SanitizedTransaction, - Transaction, TransactionError, TransactionVerificationMode, VersionedTransaction, - MAX_TX_ACCOUNT_LOCKS, + self, MessageHash, Result, SanitizedTransaction, Transaction, TransactionError, + TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS, }, transaction_context::{TransactionAccount, TransactionReturnData}, }, diff --git a/runtime/src/bank_utils.rs b/runtime/src/bank_utils.rs index 645630a89a938e..618089e10b55b7 100644 --- a/runtime/src/bank_utils.rs +++ b/runtime/src/bank_utils.rs @@ -7,7 +7,7 @@ use { solana_sdk::{pubkey::Pubkey, signature::Signer}, }; use { - solana_sdk::transaction::ExtendedSanitizedTransaction, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_svm::transaction_results::TransactionResults, solana_vote::{vote_parser, vote_sender_types::ReplayVoteSender}, }; diff --git a/runtime/src/installed_scheduler_pool.rs b/runtime/src/installed_scheduler_pool.rs index ff62b69a8d87d9..5f3679060a93e3 100644 --- a/runtime/src/installed_scheduler_pool.rs +++ b/runtime/src/installed_scheduler_pool.rs @@ -24,11 +24,8 @@ use { crate::bank::Bank, log::*, solana_program_runtime::timings::ExecuteTimings, - solana_sdk::{ - hash::Hash, - slot_history::Slot, - transaction::{ExtendedSanitizedTransaction, Result}, - }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::{hash::Hash, slot_history::Slot, transaction::Result}, std::{ fmt::Debug, ops::Deref, diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index 363379178174bd..9ff3c3d651be74 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -5,10 +5,10 @@ use { log::*, lru::LruCache, solana_measure::measure, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::{BankId, Slot}, pubkey::Pubkey, - transaction::ExtendedSanitizedTransaction, }, std::{ collections::HashMap, diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index fe1c8bc3210ea9..935b8a34a91113 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -1,7 +1,7 @@ use { crate::bank::Bank, - solana_sdk::transaction::{ExtendedSanitizedTransaction, Result}, - std::borrow::Cow, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::transaction::Result, std::borrow::Cow, }; // Represents the results of trying to lock a set of accounts diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index 2d4e2bd21018dc..b7383b4a0a454c 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -19,7 +19,6 @@ use { transaction::{Result, Transaction, TransactionError, VersionedTransaction}, }, solana_program::message::SanitizedVersionedMessage, - std::time::Instant, }; /// Maximum number of accounts that a transaction may lock. @@ -36,22 +35,6 @@ pub struct SanitizedTransaction { signatures: Vec, } -/// Sanitized transaction with option start_time -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct ExtendedSanitizedTransaction { - pub transaction: SanitizedTransaction, - pub start_time: Option, -} - -impl From for ExtendedSanitizedTransaction { - fn from(value: SanitizedTransaction) -> Self { - Self { - transaction: value, - start_time: None, - } - } -} - /// Set of accounts that must be locked for safe transaction processing #[derive(Debug, Clone, Default, Eq, PartialEq)] pub struct TransactionAccountLocks<'a> { diff --git a/svm/Cargo.toml b/svm/Cargo.toml index ca4fba3a6e2b06..b436236ac5ba0d 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -21,6 +21,7 @@ solana-loader-v4-program = { workspace = true } solana-measure = { workspace = true } solana-metrics = { workspace = true } solana-program-runtime = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-system-program = { workspace = true } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 5b9b2a1f8f0835..bf586649104510 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -10,6 +10,7 @@ use { compute_budget_processor::process_compute_budget_instructions, loaded_programs::LoadedProgramsForTxBatch, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::{ create_executable_meta, is_builtin, is_executable, Account, AccountSharedData, @@ -30,7 +31,7 @@ use { rent_debits::RentDebits, saturating_add_assign, sysvar::{self, instructions::construct_instructions_data}, - transaction::{self, ExtendedSanitizedTransaction, Result, TransactionError}, + transaction::{self, Result, TransactionError}, transaction_context::{IndexOfAccount, TransactionAccount}, }, solana_system_program::{get_system_account_kind, SystemAccountKind}, diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index e3b5de22159a1d..4b0ca0c4194899 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -26,6 +26,7 @@ use { sysvar_cache::SysvarCache, timings::{ExecuteDetailsTimings, ExecuteTimingType, ExecuteTimings}, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::{AccountSharedData, ReadableAccount, PROGRAM_OWNERS}, account_utils::StateMut, @@ -44,7 +45,7 @@ use { rent_collector::RentCollector, saturating_add_assign, timing::duration_as_us, - transaction::{self, ExtendedSanitizedTransaction, SanitizedTransaction, TransactionError}, + transaction::{self, SanitizedTransaction, TransactionError}, transaction_context::{ExecutionRecord, TransactionContext}, }, std::{ diff --git a/unified-scheduler-logic/Cargo.toml b/unified-scheduler-logic/Cargo.toml index b2e80c79c7a08f..0cc315b159af39 100644 --- a/unified-scheduler-logic/Cargo.toml +++ b/unified-scheduler-logic/Cargo.toml @@ -11,3 +11,4 @@ edition = { workspace = true } [dependencies] solana-sdk = { workspace = true } +solana-runtime-transaction = { workspace = true } diff --git a/unified-scheduler-logic/src/lib.rs b/unified-scheduler-logic/src/lib.rs index e587c3ba06f38d..e1c9dc64205a84 100644 --- a/unified-scheduler-logic/src/lib.rs +++ b/unified-scheduler-logic/src/lib.rs @@ -1,4 +1,4 @@ -use solana_sdk::transaction::ExtendedSanitizedTransaction; +use solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction; pub struct Task { transaction: ExtendedSanitizedTransaction, diff --git a/unified-scheduler-pool/Cargo.toml b/unified-scheduler-pool/Cargo.toml index 7626215b1e1126..b5a6920bb93753 100644 --- a/unified-scheduler-pool/Cargo.toml +++ b/unified-scheduler-pool/Cargo.toml @@ -17,6 +17,7 @@ log = { workspace = true } solana-ledger = { workspace = true } solana-program-runtime = { workspace = true } solana-runtime = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-unified-scheduler-logic = { workspace = true } solana-vote = { workspace = true } diff --git a/unified-scheduler-pool/src/lib.rs b/unified-scheduler-pool/src/lib.rs index 4d8780c278a3c9..6ee3460390c2a9 100644 --- a/unified-scheduler-pool/src/lib.rs +++ b/unified-scheduler-pool/src/lib.rs @@ -26,7 +26,8 @@ use { }, prioritization_fee_cache::PrioritizationFeeCache, }, - solana_sdk::transaction::{ExtendedSanitizedTransaction, Result}, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, + solana_sdk::transaction::Result, solana_unified_scheduler_logic::Task, solana_vote::vote_sender_types::ReplayVoteSender, std::{ From 02f97327367959cf51399b6b0417a3577b6c1773 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Mon, 11 Mar 2024 00:32:09 -0700 Subject: [PATCH 20/42] Fixed unit test failures --- accounts-db/src/accounts.rs | 38 ++-- core/src/banking_stage.rs | 7 +- core/src/banking_stage/consumer.rs | 39 ++-- .../forward_packet_batches_by_accounts.rs | 16 +- core/src/banking_stage/qos_service.rs | 74 ++++-- .../prio_graph_scheduler.rs | 22 +- .../scheduler_controller.rs | 16 +- .../transaction_state.rs | 11 +- .../transaction_state_container.rs | 3 +- entry/src/entry.rs | 9 +- ledger/src/blockstore_processor.rs | 14 +- rpc/src/rpc.rs | 2 +- rpc/src/transaction_status_service.rs | 2 +- runtime/src/bank/tests.rs | 10 +- runtime/src/installed_scheduler_pool.rs | 4 +- runtime/src/prioritization_fee_cache.rs | 33 +-- runtime/src/transaction_batch.rs | 11 +- svm/src/account_loader.rs | 4 +- svm/tests/account_loader.rs | 214 ++++++++++++++++++ svm/tests/rent_state.rs | 90 ++++++++ svm/tests/transaction_processor.rs | 4 +- unified-scheduler-pool/src/lib.rs | 16 +- 22 files changed, 514 insertions(+), 125 deletions(-) create mode 100644 svm/tests/account_loader.rs create mode 100644 svm/tests/rent_state.rs diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index b72b47f715394c..52674ea37824eb 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -813,7 +813,7 @@ mod tests { rent_debits::RentDebits, signature::{keypair_from_seed, signers::Signers, Keypair, Signer}, system_instruction, system_program, - transaction::{Transaction, MAX_TX_ACCOUNT_LOCKS}, + transaction::{SanitizedTransaction, Transaction, MAX_TX_ACCOUNT_LOCKS}, }, solana_svm::{ account_loader::LoadedTransaction, @@ -1080,7 +1080,7 @@ mod tests { }; let tx = new_sanitized_tx(&[&keypair], message, Hash::default()); - let results = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS); + let results = accounts.lock_accounts([tx.into()].iter(), MAX_TX_ACCOUNT_LOCKS); assert_eq!(results[0], Err(TransactionError::AccountLoadedTwice)); } @@ -1107,7 +1107,7 @@ mod tests { ..Message::default() }; - let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; + let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default()).into()]; let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); assert_eq!(results, vec![Ok(())]); accounts.unlock_accounts(txs.iter().zip(&results)); @@ -1129,7 +1129,7 @@ mod tests { ..Message::default() }; - let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default())]; + let txs = vec![new_sanitized_tx(&[&keypair], message, Hash::default()).into()]; let results = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); assert_eq!(results[0], Err(TransactionError::TooManyAccountLocks)); } @@ -1164,7 +1164,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx.clone()].iter(), MAX_TX_ACCOUNT_LOCKS); + let results0 = accounts.lock_accounts([tx.clone().into()].iter(), MAX_TX_ACCOUNT_LOCKS); assert_eq!(results0, vec![Ok(())]); assert_eq!( @@ -1198,7 +1198,7 @@ mod tests { instructions, ); let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); - let txs = vec![tx0, tx1]; + let txs = vec![tx0.into(), tx1.into()]; let results1 = accounts.lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); assert_eq!( results1, @@ -1218,7 +1218,7 @@ mod tests { 2 ); - accounts.unlock_accounts(iter::once(&tx).zip(&results0)); + accounts.unlock_accounts(iter::once(&tx.into()).zip(&results0)); accounts.unlock_accounts(txs.iter().zip(&results1)); let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; let message = Message::new_with_compiled_instructions( @@ -1230,7 +1230,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair1], message, Hash::default()); - let results2 = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS); + let results2 = accounts.lock_accounts([tx.into()].iter(), MAX_TX_ACCOUNT_LOCKS); assert_eq!( results2, vec![Ok(())] // Now keypair1 account can be locked as writable @@ -1293,7 +1293,7 @@ mod tests { let accounts_clone = accounts_arc.clone(); let exit_clone = exit.clone(); thread::spawn(move || loop { - let txs = vec![writable_tx.clone()]; + let txs = vec![writable_tx.clone().into()]; let results = accounts_clone .clone() .lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); @@ -1309,7 +1309,7 @@ mod tests { }); let counter_clone = counter; for _ in 0..5 { - let txs = vec![readonly_tx.clone()]; + let txs = vec![readonly_tx.clone().into()]; let results = accounts_arc .clone() .lock_accounts(txs.iter(), MAX_TX_ACCOUNT_LOCKS); @@ -1353,7 +1353,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx].iter(), MAX_TX_ACCOUNT_LOCKS); + let results0 = accounts.lock_accounts([tx.into()].iter(), MAX_TX_ACCOUNT_LOCKS); assert!(results0[0].is_ok()); // Instruction program-id account demoted to readonly @@ -1445,7 +1445,7 @@ mod tests { instructions, ); let tx2 = new_sanitized_tx(&[&keypair3], message, Hash::default()); - let txs = vec![tx0, tx1, tx2]; + let txs = vec![tx0.into(), tx1.into(), tx2.into()]; let qos_results = vec![ Ok(()), @@ -1575,7 +1575,7 @@ mod tests { .unwrap() .insert_new_readonly(&pubkey); } - let txs = vec![tx0.clone(), tx1.clone()]; + let txs = vec![tx0.clone().into(), tx1.clone().into()]; let execution_results = vec![new_execution_result(Ok(()), None); 2]; let (collected_accounts, transactions) = accounts.collect_accounts_to_store( &txs, @@ -1593,8 +1593,12 @@ mod tests { .any(|(pubkey, _account)| *pubkey == &keypair1.pubkey())); assert_eq!(transactions.len(), 2); - assert!(transactions.iter().any(|txn| txn.unwrap().eq(&tx0))); - assert!(transactions.iter().any(|txn| txn.unwrap().eq(&tx1))); + assert!(transactions + .iter() + .any(|txn| txn.unwrap().eq(&tx0.clone().into()))); + assert!(transactions + .iter() + .any(|txn| txn.unwrap().eq(&tx1.clone().into()))); // Ensure readonly_lock reflects lock assert_eq!( @@ -1947,7 +1951,7 @@ mod tests { let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); let accounts_db = AccountsDb::new_single_for_tests(); let accounts = Accounts::new(Arc::new(accounts_db)); - let txs = vec![tx]; + let txs = vec![tx.into()]; let execution_results = vec![new_execution_result( Err(TransactionError::InstructionError( 1, @@ -2053,7 +2057,7 @@ mod tests { let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); let accounts_db = AccountsDb::new_single_for_tests(); let accounts = Accounts::new(Arc::new(accounts_db)); - let txs = vec![tx]; + let txs = vec![tx.into()]; let execution_results = vec![new_execution_result( Err(TransactionError::InstructionError( 1, diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 603ff55f0003b4..9d30eb5a0ccaa1 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -804,6 +804,7 @@ mod tests { poh_service::PohService, }, solana_runtime::{bank::Bank, genesis_utils::bootstrap_validator_stake_lamports}, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ hash::Hash, poh_config::PohConfig, @@ -830,9 +831,11 @@ mod tests { (node, cluster_info) } - pub(crate) fn sanitize_transactions(txs: Vec) -> Vec { + pub(crate) fn sanitize_transactions( + txs: Vec, + ) -> Vec { txs.into_iter() - .map(SanitizedTransaction::from_transaction_for_tests) + .map(|t| SanitizedTransaction::from_transaction_for_tests(t).into()) .collect() } diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index b55019130c3ca1..fcd428582c4957 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -888,7 +888,7 @@ mod tests { signature::Keypair, signer::Signer, system_instruction, system_program, system_transaction, - transaction::{MessageHash, Transaction, VersionedTransaction}, + transaction::{MessageHash, SanitizedTransaction, Transaction, VersionedTransaction}, }, solana_transaction_status::{TransactionStatusMeta, VersionedTransactionWithStatusMeta}, std::{ @@ -941,7 +941,7 @@ mod tests { ); let consumer = Consumer::new(committer, recorder, QosService::new(1), None); let process_transactions_summary = - consumer.process_transactions(&bank, &Instant::now(), &transactions); + consumer.process_transactions(&bank, &Instant::now(), &transactions, &mut None); poh_recorder .read() @@ -1117,7 +1117,7 @@ mod tests { let consumer = Consumer::new(committer, recorder, QosService::new(1), None); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); let ExecuteAndCommitTransactionsOutput { transactions_attempted_execution_count, @@ -1162,7 +1162,7 @@ mod tests { )]); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); let ExecuteAndCommitTransactionsOutput { transactions_attempted_execution_count, @@ -1302,7 +1302,7 @@ mod tests { let consumer = Consumer::new(committer, recorder, QosService::new(1), None); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); let ExecuteAndCommitTransactionsOutput { transactions_attempted_execution_count, executed_transactions_count, @@ -1404,7 +1404,7 @@ mod tests { let consumer = Consumer::new(committer, recorder, QosService::new(1), None); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); let ExecuteAndCommitTransactionsOutput { transactions_attempted_execution_count, @@ -1510,7 +1510,7 @@ mod tests { )]); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); let ExecuteAndCommitTransactionsOutput { executed_with_successful_result_count, @@ -1541,7 +1541,7 @@ mod tests { ]); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); let ExecuteAndCommitTransactionsOutput { executed_with_successful_result_count, @@ -1575,14 +1575,17 @@ mod tests { } }; - let mut cost = CostModel::calculate_cost(&transactions[0], &bank.feature_set); + let mut cost = + CostModel::calculate_cost(&transactions[0].transaction, &bank.feature_set); if let TransactionCost::Transaction(ref mut usage_cost) = cost { usage_cost.programs_execution_cost = actual_programs_execution_cost; } block_cost + cost.sum() } else { - block_cost + CostModel::calculate_cost(&transactions[0], &bank.feature_set).sum() + block_cost + + CostModel::calculate_cost(&transactions[0].transaction, &bank.feature_set) + .sum() }; assert_eq!(get_block_cost(), expected_block_cost); @@ -1650,7 +1653,7 @@ mod tests { let consumer = Consumer::new(committer, recorder, QosService::new(1), None); let process_transactions_batch_output = - consumer.process_and_record_transactions(&bank, &transactions, 0); + consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); poh_recorder .read() @@ -1847,7 +1850,7 @@ mod tests { let consumer = Consumer::new(committer, recorder.clone(), QosService::new(1), None); let process_transactions_summary = - consumer.process_transactions(&bank, &Instant::now(), &transactions); + consumer.process_transactions(&bank, &Instant::now(), &transactions, &mut None); let ProcessTransactionsSummary { reached_max_poh_height, @@ -1974,7 +1977,7 @@ mod tests { ); let consumer = Consumer::new(committer, recorder, QosService::new(1), None); - let _ = consumer.process_and_record_transactions(&bank, &transactions, 0); + let _ = consumer.process_and_record_transactions(&bank, &transactions, 0, &mut None); drop(consumer); // drop/disconnect transaction_status_sender transaction_status_service.join().unwrap(); @@ -2119,7 +2122,12 @@ mod tests { ); let consumer = Consumer::new(committer, recorder, QosService::new(1), None); - let _ = consumer.process_and_record_transactions(&bank, &[sanitized_tx.clone()], 0); + let _ = consumer.process_and_record_transactions( + &bank, + &[sanitized_tx.clone().into()], + 0, + &mut None, + ); drop(consumer); // drop/disconnect transaction_status_sender transaction_status_service.join().unwrap(); @@ -2328,7 +2336,8 @@ mod tests { &lock_account, 1, bank.last_blockhash(), - )); + )) + .into(); let _ = bank_start.working_bank.accounts().lock_accounts( std::iter::once(&manual_lock_tx), bank_start.working_bank.get_transaction_account_lock_limit(), diff --git a/core/src/banking_stage/forward_packet_batches_by_accounts.rs b/core/src/banking_stage/forward_packet_batches_by_accounts.rs index 6b1685c8a46483..3db9797efcad8b 100644 --- a/core/src/banking_stage/forward_packet_batches_by_accounts.rs +++ b/core/src/banking_stage/forward_packet_batches_by_accounts.rs @@ -139,8 +139,12 @@ mod tests { super::*, crate::banking_stage::unprocessed_packet_batches::DeserializedPacket, solana_sdk::{ - compute_budget::ComputeBudgetInstruction, feature_set::FeatureSet, message::Message, - pubkey::Pubkey, system_instruction, transaction::Transaction, + compute_budget::ComputeBudgetInstruction, + feature_set::FeatureSet, + message::Message, + pubkey::Pubkey, + system_instruction, + transaction::{SanitizedTransaction, Transaction}, }, }; @@ -149,7 +153,7 @@ mod tests { fn build_test_transaction_and_packet( priority: u64, write_to_account: &Pubkey, - ) -> (SanitizedTransaction, DeserializedPacket, u32) { + ) -> (ExtendedSanitizedTransaction, DeserializedPacket, u32) { let from_account = solana_sdk::pubkey::new_rand(); let transaction = Transaction::new_unsigned(Message::new( @@ -169,7 +173,11 @@ mod tests { // set limit ratio so each batch can only have one test transaction let limit_ratio: u32 = ((block_cost_limits::MAX_WRITABLE_ACCOUNT_UNITS - cost + 1) / cost) as u32; - (sanitized_transaction, deserialized_packet, limit_ratio) + ( + sanitized_transaction.into(), + deserialized_packet, + limit_ratio, + ) } #[test] diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 91b89a9e619cc1..78cd5bfd8109c8 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -582,6 +582,7 @@ mod tests { hash::Hash, signature::{Keypair, Signer}, system_transaction, + transaction::SanitizedTransaction, }, solana_vote_program::vote_transaction, std::sync::Arc, @@ -593,20 +594,27 @@ mod tests { // make a vec of txs let keypair = Keypair::new(); - let transfer_tx = SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&keypair, &keypair.pubkey(), 1, Hash::default()), - ); - let vote_tx = SanitizedTransaction::from_transaction_for_tests( - vote_transaction::new_vote_transaction( - vec![42], - Hash::default(), - Hash::default(), - &keypair, + let transfer_tx: ExtendedSanitizedTransaction = + SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( &keypair, - &keypair, - None, - ), - ); + &keypair.pubkey(), + 1, + Hash::default(), + )) + .into(); + let vote_tx: ExtendedSanitizedTransaction = + SanitizedTransaction::from_transaction_for_tests( + vote_transaction::new_vote_transaction( + vec![42], + Hash::default(), + Hash::default(), + &keypair, + &keypair, + &keypair, + None, + ), + ) + .into(); let txs = vec![transfer_tx.clone(), vote_tx.clone(), vote_tx, transfer_tx]; let qos_service = QosService::new(1); @@ -624,7 +632,8 @@ mod tests { .map(|(index, cost)| { assert_eq!( cost.as_ref().unwrap().sum(), - CostModel::calculate_cost(&txs[index], &FeatureSet::all_enabled()).sum() + CostModel::calculate_cost(&txs[index].transaction, &FeatureSet::all_enabled()) + .sum() ); }) .collect_vec(); @@ -656,7 +665,12 @@ mod tests { let vote_tx_cost = CostModel::calculate_cost(&vote_tx, &FeatureSet::all_enabled()).sum(); // make a vec of txs - let txs = vec![transfer_tx.clone(), vote_tx.clone(), transfer_tx, vote_tx]; + let txs = vec![ + transfer_tx.clone().into(), + vote_tx.clone().into(), + transfer_tx.into(), + vote_tx.into(), + ]; let qos_service = QosService::new(1); let txs_costs = qos_service.compute_transaction_costs( @@ -695,8 +709,8 @@ mod tests { let transfer_tx = SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(&keypair, &keypair.pubkey(), 1, Hash::default()), ); - let txs: Vec = (0..transaction_count) - .map(|_| transfer_tx.clone()) + let txs: Vec = (0..transaction_count) + .map(|_| transfer_tx.clone().into()) .collect(); let execute_units_adjustment = 10u64; @@ -761,10 +775,15 @@ mod tests { // calculate their costs, apply to cost_tracker let transaction_count = 5; let keypair = Keypair::new(); - let transfer_tx = SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&keypair, &keypair.pubkey(), 1, Hash::default()), - ); - let txs: Vec = (0..transaction_count) + let transfer_tx: ExtendedSanitizedTransaction = + SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( + &keypair, + &keypair.pubkey(), + 1, + Hash::default(), + )) + .into(); + let txs: Vec = (0..transaction_count) .map(|_| transfer_tx.clone()) .collect(); @@ -814,10 +833,15 @@ mod tests { // calculate their costs, apply to cost_tracker let transaction_count = 5; let keypair = Keypair::new(); - let transfer_tx = SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&keypair, &keypair.pubkey(), 1, Hash::default()), - ); - let txs: Vec = (0..transaction_count) + let transfer_tx: ExtendedSanitizedTransaction = + SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( + &keypair, + &keypair.pubkey(), + 1, + Hash::default(), + )) + .into(); + let txs: Vec = (0..transaction_count) .map(|_| transfer_tx.clone()) .collect(); let execute_units_adjustment = 10u64; 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 a80cdabb668ea6..59bc13c6fe29a6 100644 --- a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs +++ b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs @@ -490,8 +490,14 @@ mod tests { crossbeam_channel::{unbounded, Receiver}, itertools::Itertools, solana_sdk::{ - compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, pubkey::Pubkey, - signature::Keypair, signer::Signer, system_instruction, transaction::Transaction, + compute_budget::ComputeBudgetInstruction, + hash::Hash, + message::Message, + pubkey::Pubkey, + signature::Keypair, + signer::Signer, + system_instruction, + transaction::{SanitizedTransaction, Transaction}, }, std::borrow::Borrow, }; @@ -567,7 +573,8 @@ mod tests { to_pubkeys, lamports, compute_unit_price, - ); + ) + .into(); let transaction_ttl = SanitizedTransactionTTL { transaction, max_age_slot: Slot::MAX, @@ -596,11 +603,11 @@ mod tests { .unzip() } - fn test_pre_graph_filter(_txs: &[&SanitizedTransaction], results: &mut [bool]) { + fn test_pre_graph_filter(_txs: &[&ExtendedSanitizedTransaction], results: &mut [bool]) { results.fill(true); } - fn test_pre_lock_filter(_tx: &SanitizedTransaction) -> bool { + fn test_pre_lock_filter(_tx: &ExtendedSanitizedTransaction) -> bool { true } @@ -775,8 +782,9 @@ mod tests { ]); // 2nd transaction should be filtered out and dropped before locking. - let pre_lock_filter = - |tx: &SanitizedTransaction| tx.message().fee_payer() != &keypair.pubkey(); + let pre_lock_filter = |tx: &ExtendedSanitizedTransaction| { + tx.transaction.message().fee_payer() != &keypair.pubkey() + }; let scheduling_summary = scheduler .schedule(&mut container, test_pre_graph_filter, pre_lock_filter) .unwrap(); diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index e3aa895fddc54c..b5a7a786424ec9 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -749,7 +749,7 @@ mod tests { let message_hashes = consume_work .transactions .iter() - .map(|tx| tx.message_hash()) + .map(|tx| tx.transaction.message_hash()) .collect_vec(); assert_eq!(message_hashes, vec![&tx2_hash, &tx1_hash]); } @@ -807,7 +807,11 @@ mod tests { let num_txs_per_batch = consume_works.iter().map(|cw| cw.ids.len()).collect_vec(); let message_hashes = consume_works .iter() - .flat_map(|cw| cw.transactions.iter().map(|tx| tx.message_hash())) + .flat_map(|cw| { + cw.transactions + .iter() + .map(|tx| tx.transaction.message_hash()) + }) .collect_vec(); assert_eq!(num_txs_per_batch, vec![1; 2]); assert_eq!(message_hashes, vec![&tx2_hash, &tx1_hash]); @@ -930,14 +934,14 @@ mod tests { .unwrap() .transactions .iter() - .map(|tx| *tx.message_hash()) + .map(|tx| *tx.transaction.message_hash()) .collect_vec(); let t1_actual = consume_work_receivers[1] .try_recv() .unwrap() .transactions .iter() - .map(|tx| *tx.message_hash()) + .map(|tx| *tx.transaction.message_hash()) .collect_vec(); assert_eq!(t0_actual, t0_expected); @@ -996,7 +1000,7 @@ mod tests { let message_hashes = consume_work .transactions .iter() - .map(|tx| tx.message_hash()) + .map(|tx| tx.transaction.message_hash()) .collect_vec(); assert_eq!(message_hashes, vec![&tx2_hash, &tx1_hash]); @@ -1016,7 +1020,7 @@ mod tests { let message_hashes = consume_work .transactions .iter() - .map(|tx| tx.message_hash()) + .map(|tx| tx.transaction.message_hash()) .collect_vec(); assert_eq!(message_hashes, vec![&tx1_hash]); } diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state.rs b/core/src/banking_stage/transaction_scheduler/transaction_state.rs index 22bf8a7b87344a..642bed2a7639bd 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state.rs @@ -141,8 +141,13 @@ mod tests { use { super::*, solana_sdk::{ - compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message, - signature::Keypair, signer::Signer, system_instruction, transaction::Transaction, + compute_budget::ComputeBudgetInstruction, + hash::Hash, + message::Message, + signature::Keypair, + signer::Signer, + system_instruction, + transaction::{SanitizedTransaction, Transaction}, }, }; @@ -160,7 +165,7 @@ mod tests { let tx = Transaction::new(&[&from_keypair], message, Hash::default()); let transaction_ttl = SanitizedTransactionTTL { - transaction: SanitizedTransaction::from_transaction_for_tests(tx), + transaction: SanitizedTransaction::from_transaction_for_tests(tx).into(), max_age_slot: Slot::MAX, }; const TEST_TRANSACTION_COST: u64 = 5000; 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 a627375a03e6ba..23873da4c44167 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs @@ -187,7 +187,8 @@ mod tests { &[&from_keypair], message, Hash::default(), - )); + )) + .into(); let transaction_ttl = SanitizedTransactionTTL { transaction: tx, max_age_slot: Slot::MAX, diff --git a/entry/src/entry.rs b/entry/src/entry.rs index 3ea1ca7e51551a..2a279408324d83 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -982,7 +982,7 @@ mod tests { dyn Fn( VersionedTransaction, TransactionVerificationMode, - ) -> Result + ) -> Result + Send + Sync, >, @@ -994,7 +994,7 @@ mod tests { } else { TransactionVerificationMode::FullVerification }; - move |versioned_tx: VersionedTransaction| -> Result { + move |versioned_tx: VersionedTransaction| -> Result { verify(versioned_tx, verification_mode) } }; @@ -1035,7 +1035,7 @@ mod tests { let verify_transaction = { move |versioned_tx: VersionedTransaction, verification_mode: TransactionVerificationMode| - -> Result { + -> Result { let sanitized_tx = { let message_hash = if verification_mode == TransactionVerificationMode::FullVerification { @@ -1050,7 +1050,8 @@ mod tests { None, SimpleAddressLoader::Disabled, ) - }?; + }? + .into(); Ok(sanitized_tx) } diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index b9f85dab6a0660..3182fb9d2ea635 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -1972,7 +1972,7 @@ pub mod tests { signature::{Keypair, Signer}, system_instruction::SystemError, system_transaction, - transaction::{Transaction, TransactionError}, + transaction::{SanitizedTransaction, Transaction, TransactionError}, }, solana_svm::transaction_processor::ExecutionRecordingConfig, solana_vote::vote_account::VoteAccount, @@ -3968,6 +3968,7 @@ pub mod tests { ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, + None, ); let (err, signature) = get_first_error(&batch, fee_collection_results).unwrap(); assert_eq!(err.unwrap_err(), TransactionError::AccountNotFound); @@ -4361,7 +4362,7 @@ pub mod tests { fn create_test_transactions( mint_keypair: &Keypair, genesis_hash: &Hash, - ) -> Vec { + ) -> Vec { let pubkey = solana_sdk::pubkey::new_rand(); let keypair2 = Keypair::new(); let pubkey2 = solana_sdk::pubkey::new_rand(); @@ -4374,19 +4375,22 @@ pub mod tests { &pubkey, 1, *genesis_hash, - )), + )) + .into(), SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( &keypair2, &pubkey2, 1, *genesis_hash, - )), + )) + .into(), SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( &keypair3, &pubkey3, 1, *genesis_hash, - )), + )) + .into(), ] } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 7b8a09c37da55f..2ebcdc9f883afb 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -5081,7 +5081,7 @@ pub mod tests { let prioritization_fee_cache = &self.meta.prioritization_fee_cache; let transactions: Vec<_> = transactions .into_iter() - .map(SanitizedTransaction::from_transaction_for_tests) + .map(|t| SanitizedTransaction::from_transaction_for_tests(t).into()) .collect(); prioritization_fee_cache.update(&bank, transactions.iter()); } diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 29a20542379ba3..055daeea04e976 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -415,7 +415,7 @@ pub(crate) mod tests { let transaction_index: usize = bank.transaction_count().try_into().unwrap(); let transaction_status_batch = TransactionStatusBatch { bank, - transactions: vec![transaction], + transactions: vec![transaction.into()], execution_results: vec![transaction_result], balances, token_balances, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 29dbdc2e5aeacd..c7e3c68e74f793 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -2903,7 +2903,7 @@ fn test_filter_program_errors_and_collect_fee() { ]; let initial_balance = bank.get_balance(&leader); - let results = bank.filter_program_errors_and_collect_fee(&[tx1, tx2], &results); + let results = bank.filter_program_errors_and_collect_fee(&[tx1.into(), tx2.into()], &results); bank.freeze(); assert_eq!( bank.get_balance(&leader), @@ -2954,7 +2954,7 @@ fn test_filter_program_errors_and_collect_compute_unit_fee() { ]; let initial_balance = bank.get_balance(&leader); - let results = bank.filter_program_errors_and_collect_fee(&[tx1, tx2], &results); + let results = bank.filter_program_errors_and_collect_fee(&[tx1.into(), tx2.into()], &results); bank.freeze(); assert_eq!( bank.get_balance(&leader), @@ -3105,6 +3105,7 @@ fn test_interleaving_locks() { ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, + None, ) .0 .fee_collection_results; @@ -5929,6 +5930,7 @@ fn test_pre_post_transaction_balances() { ExecutionRecordingConfig::new_single_setting(false), &mut ExecuteTimings::default(), None, + None, ); assert_eq!(transaction_balances_set.pre_balances.len(), 3); @@ -9213,6 +9215,7 @@ fn test_tx_log_order() { }, &mut ExecuteTimings::default(), None, + None, ) .0 .execution_results; @@ -9323,6 +9326,7 @@ fn test_tx_return_data() { }, &mut ExecuteTimings::default(), None, + None, ) .0 .execution_results[0] @@ -13790,7 +13794,7 @@ fn test_failed_simulation_compute_units() { let transaction = Transaction::new(&[&mint_keypair], message, bank.last_blockhash()); bank.freeze(); - let sanitized = SanitizedTransaction::from_transaction_for_tests(transaction); + let sanitized = SanitizedTransaction::from_transaction_for_tests(transaction).into(); let simulation = bank.simulate_transaction(&sanitized, false); assert_eq!(expected_consumed_units, simulation.units_consumed); } diff --git a/runtime/src/installed_scheduler_pool.rs b/runtime/src/installed_scheduler_pool.rs index 5f3679060a93e3..df77af1718d934 100644 --- a/runtime/src/installed_scheduler_pool.rs +++ b/runtime/src/installed_scheduler_pool.rs @@ -427,7 +427,7 @@ mod tests { }, assert_matches::assert_matches, mockall::Sequence, - solana_sdk::system_transaction, + solana_sdk::{system_transaction, transaction::SanitizedTransaction}, std::sync::Mutex, }; @@ -574,6 +574,6 @@ mod tests { ); let bank = BankWithScheduler::new(bank, Some(mocked_scheduler)); - bank.schedule_transaction_executions([(&tx0, &0)].into_iter()); + bank.schedule_transaction_executions([(&tx0.into(), &0)].into_iter()); } } diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index 9ff3c3d651be74..37800293db68e3 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -475,7 +475,7 @@ mod tests { fn sync_update<'a>( prioritization_fee_cache: &PrioritizationFeeCache, bank: Arc, - txs: impl Iterator + ExactSizeIterator, + txs: impl Iterator + ExactSizeIterator, ) { let expected_update_count = prioritization_fee_cache .metrics @@ -534,9 +534,9 @@ mod tests { // [2, a, c ] --> [2, 2, 5, 2 ] // let txs = vec![ - build_sanitized_transaction_for_test(5, &write_account_a, &write_account_b), - build_sanitized_transaction_for_test(9, &write_account_b, &write_account_c), - build_sanitized_transaction_for_test(2, &write_account_a, &write_account_c), + build_sanitized_transaction_for_test(5, &write_account_a, &write_account_b).into(), + build_sanitized_transaction_for_test(9, &write_account_b, &write_account_c).into(), + build_sanitized_transaction_for_test(2, &write_account_a, &write_account_c).into(), ]; let bank = Arc::new(Bank::default_for_tests()); @@ -651,12 +651,13 @@ mod tests { // Assert after add one transaction for slot 1 { let txs = vec![ - build_sanitized_transaction_for_test(2, &write_account_a, &write_account_b), + build_sanitized_transaction_for_test(2, &write_account_a, &write_account_b).into(), build_sanitized_transaction_for_test( 1, &Pubkey::new_unique(), &Pubkey::new_unique(), - ), + ) + .into(), ]; sync_update(&prioritization_fee_cache, bank1.clone(), txs.iter()); // before block is marked as completed @@ -714,12 +715,13 @@ mod tests { // Assert after add one transaction for slot 2 { let txs = vec![ - build_sanitized_transaction_for_test(4, &write_account_b, &write_account_c), + build_sanitized_transaction_for_test(4, &write_account_b, &write_account_c).into(), build_sanitized_transaction_for_test( 3, &Pubkey::new_unique(), &Pubkey::new_unique(), - ), + ) + .into(), ]; sync_update(&prioritization_fee_cache, bank2.clone(), txs.iter()); // before block is marked as completed @@ -788,12 +790,13 @@ mod tests { // Assert after add one transaction for slot 3 { let txs = vec![ - build_sanitized_transaction_for_test(6, &write_account_a, &write_account_c), + build_sanitized_transaction_for_test(6, &write_account_a, &write_account_c).into(), build_sanitized_transaction_for_test( 5, &Pubkey::new_unique(), &Pubkey::new_unique(), - ), + ) + .into(), ]; sync_update(&prioritization_fee_cache, bank3.clone(), txs.iter()); // before block is marked as completed @@ -883,12 +886,13 @@ mod tests { // Assert after add transactions for bank1 of slot 1 { let txs = vec![ - build_sanitized_transaction_for_test(2, &write_account_a, &write_account_b), + build_sanitized_transaction_for_test(2, &write_account_a, &write_account_b).into(), build_sanitized_transaction_for_test( 1, &Pubkey::new_unique(), &Pubkey::new_unique(), - ), + ) + .into(), ]; sync_update(&prioritization_fee_cache, bank1.clone(), txs.iter()); @@ -903,12 +907,13 @@ mod tests { // Assert after add transactions for bank2 of slot 1 { let txs = vec![ - build_sanitized_transaction_for_test(4, &write_account_b, &write_account_c), + build_sanitized_transaction_for_test(4, &write_account_b, &write_account_c).into(), build_sanitized_transaction_for_test( 3, &Pubkey::new_unique(), &Pubkey::new_unique(), - ), + ) + .into(), ]; sync_update(&prioritization_fee_cache, bank2.clone(), txs.iter()); diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index 935b8a34a91113..9ab4be21047682 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -100,7 +100,7 @@ mod tests { use { super::*, crate::genesis_utils::{create_genesis_config_with_leader, GenesisConfigInfo}, - solana_sdk::{signature::Keypair, system_transaction, transaction::TransactionError}, + solana_sdk::{signature::Keypair, system_transaction, transaction::{SanitizedTransaction, TransactionError}}, }; #[test] @@ -172,7 +172,7 @@ mod tests { ); } - fn setup(insert_conflicting_tx: bool) -> (Bank, Vec) { + fn setup(insert_conflicting_tx: bool) -> (Bank, Vec) { let dummy_leader_pubkey = solana_sdk::pubkey::new_rand(); let GenesisConfigInfo { genesis_config, @@ -187,16 +187,15 @@ mod tests { let mut txs = vec![SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(&mint_keypair, &pubkey, 1, genesis_config.hash()), - )]; + ).into()]; if insert_conflicting_tx { txs.push(SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(&mint_keypair, &pubkey2, 1, genesis_config.hash()), - )); + ).into()); } txs.push(SanitizedTransaction::from_transaction_for_tests( system_transaction::transfer(&keypair2, &pubkey2, 1, genesis_config.hash()), - )); - + ).into()); (bank, txs) } } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index bf586649104510..a9ef7eb4db9d19 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -550,7 +550,7 @@ mod tests { fee_structure: &FeeStructure, ) -> Vec { feature_set.deactivate(&feature_set::disable_rent_fees_collection::id()); - let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); + let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx).into(); let mut accounts_map = HashMap::new(); for (pubkey, account) in ka { accounts_map.insert(*pubkey, account.clone()); @@ -1026,7 +1026,7 @@ mod tests { tx: Transaction, account_overrides: Option<&AccountOverrides>, ) -> Vec { - let tx = SanitizedTransaction::from_transaction_for_tests(tx); + let tx = SanitizedTransaction::from_transaction_for_tests(tx).into(); let mut error_counters = TransactionErrorMetrics::default(); let mut accounts_map = HashMap::new(); diff --git a/svm/tests/account_loader.rs b/svm/tests/account_loader.rs new file mode 100644 index 00000000000000..f23f2def69f909 --- /dev/null +++ b/svm/tests/account_loader.rs @@ -0,0 +1,214 @@ +use { + crate::mock_bank::MockBankCallback, + solana_program_runtime::loaded_programs::LoadedProgramsForTxBatch, + solana_sdk::{ + account::{AccountSharedData, WritableAccount}, + fee::FeeStructure, + hash::Hash, + instruction::CompiledInstruction, + message::{LegacyMessage, Message, MessageHeader, SanitizedMessage}, + native_loader, + nonce_info::{NonceFull, NoncePartial}, + pubkey::Pubkey, + rent_collector::RENT_EXEMPT_RENT_EPOCH, + rent_debits::RentDebits, + signature::{Keypair, Signature, Signer}, + transaction::{SanitizedTransaction, TransactionError}, + }, + solana_svm::{ + account_loader::{load_accounts, LoadedTransaction, TransactionCheckResult}, + transaction_error_metrics::TransactionErrorMetrics, + }, + std::collections::HashMap, +}; + +mod mock_bank; + +#[test] +fn test_load_accounts_success() { + let key1 = Keypair::new(); + let key2 = Keypair::new(); + let key3 = Keypair::new(); + let key4 = Keypair::new(); + + let message = Message { + account_keys: vec![key2.pubkey(), key1.pubkey(), key4.pubkey()], + header: MessageHeader::default(), + instructions: vec![ + CompiledInstruction { + program_id_index: 1, + accounts: vec![0], + data: vec![], + }, + CompiledInstruction { + program_id_index: 1, + accounts: vec![2], + data: vec![], + }, + ], + recent_blockhash: Hash::default(), + }; + + let legacy = LegacyMessage::new(message); + let sanitized_message = SanitizedMessage::Legacy(legacy); + let mut mock_bank = MockBankCallback::default(); + let mut account_data = AccountSharedData::default(); + account_data.set_executable(true); + account_data.set_owner(key3.pubkey()); + mock_bank + .account_shared_data + .insert(key1.pubkey(), account_data); + + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(200); + mock_bank + .account_shared_data + .insert(key2.pubkey(), account_data); + + let mut account_data = AccountSharedData::default(); + account_data.set_executable(true); + account_data.set_owner(native_loader::id()); + mock_bank + .account_shared_data + .insert(key3.pubkey(), account_data); + + let mut error_counter = TransactionErrorMetrics::default(); + let loaded_programs = LoadedProgramsForTxBatch::default(); + + let sanitized_transaction = SanitizedTransaction::new_for_tests( + sanitized_message, + vec![Signature::new_unique()], + false, + ); + let lock_results = + (Ok(()), Some(NoncePartial::default()), Some(20u64)) as TransactionCheckResult; + + let results = load_accounts( + &mock_bank, + &[sanitized_transaction.into()], + &[lock_results], + &mut error_counter, + &FeeStructure::default(), + None, + &HashMap::new(), + &loaded_programs, + ); + + let mut account_data = AccountSharedData::default(); + account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + + assert_eq!(results.len(), 1); + let (loaded_result, nonce) = results[0].clone(); + assert_eq!( + loaded_result.unwrap(), + LoadedTransaction { + accounts: vec![ + ( + key2.pubkey(), + mock_bank.account_shared_data[&key2.pubkey()].clone() + ), + ( + key1.pubkey(), + mock_bank.account_shared_data[&key1.pubkey()].clone() + ), + (key4.pubkey(), account_data), + ( + key3.pubkey(), + mock_bank.account_shared_data[&key3.pubkey()].clone() + ), + ], + program_indices: vec![vec![3, 1], vec![3, 1]], + rent: 0, + rent_debits: RentDebits::default() + } + ); + + assert_eq!( + nonce.unwrap(), + NonceFull::new( + Pubkey::from([0; 32]), + AccountSharedData::default(), + Some(mock_bank.account_shared_data[&key2.pubkey()].clone()) + ) + ); +} + +#[test] +fn test_load_accounts_error() { + let mock_bank = MockBankCallback::default(); + let message = Message { + account_keys: vec![Pubkey::new_from_array([0; 32])], + header: MessageHeader::default(), + instructions: vec![CompiledInstruction { + program_id_index: 0, + accounts: vec![], + data: vec![], + }], + recent_blockhash: Hash::default(), + }; + + let legacy = LegacyMessage::new(message); + let sanitized_message = SanitizedMessage::Legacy(legacy); + let sanitized_transaction = SanitizedTransaction::new_for_tests( + sanitized_message, + vec![Signature::new_unique()], + false, + ); + + let lock_results = (Ok(()), Some(NoncePartial::default()), None) as TransactionCheckResult; + let fee_structure = FeeStructure::default(); + + let result = load_accounts( + &mock_bank, + &[sanitized_transaction.clone().into()], + &[lock_results], + &mut TransactionErrorMetrics::default(), + &fee_structure, + None, + &HashMap::new(), + &LoadedProgramsForTxBatch::default(), + ); + + assert_eq!( + result, + vec![(Err(TransactionError::BlockhashNotFound), None)] + ); + + let lock_results = + (Ok(()), Some(NoncePartial::default()), Some(20u64)) as TransactionCheckResult; + + let result = load_accounts( + &mock_bank, + &[sanitized_transaction.clone().into()], + &[lock_results.clone()], + &mut TransactionErrorMetrics::default(), + &fee_structure, + None, + &HashMap::new(), + &LoadedProgramsForTxBatch::default(), + ); + + assert_eq!(result, vec![(Err(TransactionError::AccountNotFound), None)]); + + let lock_results = ( + Err(TransactionError::InvalidWritableAccount), + Some(NoncePartial::default()), + Some(20u64), + ) as TransactionCheckResult; + + let result = load_accounts( + &mock_bank, + &[sanitized_transaction.clone().into()], + &[lock_results], + &mut TransactionErrorMetrics::default(), + &fee_structure, + None, + &HashMap::new(), + &LoadedProgramsForTxBatch::default(), + ); + + assert_eq!( + result, + vec![(Err(TransactionError::InvalidWritableAccount), None)] + ); +} diff --git a/svm/tests/rent_state.rs b/svm/tests/rent_state.rs new file mode 100644 index 00000000000000..d958f6f2f12c61 --- /dev/null +++ b/svm/tests/rent_state.rs @@ -0,0 +1,90 @@ +#![cfg(test)] + +use { + solana_program_runtime::{ + compute_budget::ComputeBudget, compute_budget_processor, + loaded_programs::LoadedProgramsForTxBatch, + }, + solana_sdk::{ + account::{AccountSharedData, WritableAccount}, + fee::FeeStructure, + hash::Hash, + native_loader, + native_token::sol_to_lamports, + pubkey::Pubkey, + rent::Rent, + signature::{Keypair, Signer}, + system_transaction, + transaction::SanitizedTransaction, + transaction_context::TransactionContext, + }, + solana_svm::{ + account_loader::load_accounts, transaction_account_state_info::TransactionAccountStateInfo, + transaction_error_metrics::TransactionErrorMetrics, + }, + std::collections::HashMap, +}; + +mod mock_bank; + +#[test] +fn test_rent_state_list_len() { + let mint_keypair = Keypair::new(); + let mut bank = mock_bank::MockBankCallback::default(); + let recipient = Pubkey::new_unique(); + let last_block_hash = Hash::new_unique(); + + let mut system_data = AccountSharedData::default(); + system_data.set_executable(true); + system_data.set_owner(native_loader::id()); + bank.account_shared_data + .insert(Pubkey::new_from_array([0u8; 32]), system_data); + + let mut mint_data = AccountSharedData::default(); + mint_data.set_lamports(2); + bank.account_shared_data + .insert(mint_keypair.pubkey(), mint_data); + + bank.account_shared_data + .insert(recipient, AccountSharedData::default()); + + let tx = system_transaction::transfer( + &mint_keypair, + &recipient, + sol_to_lamports(1.), + last_block_hash, + ); + let num_accounts = tx.message().account_keys.len(); + let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); + let mut error_counters = TransactionErrorMetrics::default(); + let loaded_txs = load_accounts( + &bank, + &[sanitized_tx.clone().into()], + &[(Ok(()), None, Some(0))], + &mut error_counters, + &FeeStructure::default(), + None, + &HashMap::new(), + &LoadedProgramsForTxBatch::default(), + ); + + let compute_budget = ComputeBudget::new(u64::from( + compute_budget_processor::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, + )); + let transaction_context = TransactionContext::new( + loaded_txs[0].0.as_ref().unwrap().accounts.clone(), + Rent::default(), + compute_budget.max_invoke_stack_height, + compute_budget.max_instruction_trace_length, + ); + + assert_eq!( + TransactionAccountStateInfo::new( + &Rent::default(), + &transaction_context, + sanitized_tx.message() + ) + .len(), + num_accounts, + ); +} diff --git a/svm/tests/transaction_processor.rs b/svm/tests/transaction_processor.rs index 1704054246748d..ff8e543c4d5c5e 100644 --- a/svm/tests/transaction_processor.rs +++ b/svm/tests/transaction_processor.rs @@ -95,7 +95,7 @@ fn test_filter_executable_program_accounts() { let owners = &[program1_pubkey, program2_pubkey]; let programs = TransactionBatchProcessor::::filter_executable_program_accounts( &bank, - &[sanitized_tx1, sanitized_tx2], + &[sanitized_tx1.into(), sanitized_tx2.into()], &mut [(Ok(()), None, Some(0)), (Ok(()), None, Some(0))], owners, ); @@ -189,7 +189,7 @@ fn test_filter_executable_program_accounts_invalid_blockhash() { let mut lock_results = vec![(Ok(()), None, Some(0)), (Ok(()), None, None)]; let programs = TransactionBatchProcessor::::filter_executable_program_accounts( &bank, - &[sanitized_tx1, sanitized_tx2], + &[sanitized_tx1.into(), sanitized_tx2.into()], &mut lock_results, owners, ); diff --git a/unified-scheduler-pool/src/lib.rs b/unified-scheduler-pool/src/lib.rs index 6ee3460390c2a9..8821c28257d0f3 100644 --- a/unified-scheduler-pool/src/lib.rs +++ b/unified-scheduler-pool/src/lib.rs @@ -963,7 +963,8 @@ mod tests { &solana_sdk::pubkey::new_rand(), 2, genesis_config.hash(), - )); + )) + .into(); let bank = Bank::new_for_tests(&genesis_config); let bank = setup_dummy_fork_graph(bank); let ignored_prioritization_fee_cache = Arc::new(PrioritizationFeeCache::new(0u64)); @@ -1004,7 +1005,8 @@ mod tests { &solana_sdk::pubkey::new_rand(), 2, genesis_config.hash(), - )); + )) + .into(); assert_eq!(bank.transaction_count(), 0); scheduler.schedule_execution(&(bad_tx, 0)); // simulate the task-sending thread is stalled for some reason. @@ -1017,7 +1019,8 @@ mod tests { &solana_sdk::pubkey::new_rand(), 3, genesis_config.hash(), - )); + )) + .into(); // make sure this tx is really a good one to execute. assert_matches!( bank.simulate_transaction_unchecked(good_tx_after_bad_tx, false) @@ -1080,7 +1083,10 @@ mod tests { &self.2 } - fn schedule_execution(&self, &(transaction, index): &(&SanitizedTransaction, usize)) { + fn schedule_execution( + &self, + &(transaction, index): &(&ExtendedSanitizedTransaction, usize), + ) { let transaction_and_index = (transaction.clone(), index); let context = self.context().clone(); let pool = self.3.clone(); @@ -1208,7 +1214,7 @@ mod tests { assert_eq!(bank.transaction_count(), 0); // schedule but not immediately execute transaction - bank.schedule_transaction_executions([(&very_old_valid_tx, &0)].into_iter()); + bank.schedule_transaction_executions([(&very_old_valid_tx.into(), &0)].into_iter()); // this calls register_recent_blockhash internally bank.fill_bank_with_ticks_for_tests(); From e92833a7884581e27d3b20921bda6864042fd462 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Mon, 11 Mar 2024 01:10:02 -0700 Subject: [PATCH 21/42] Fixed unit test failures and merge issues --- runtime/src/bank.rs | 2 +- runtime/src/transaction_batch.rs | 43 +++++-- svm/src/account_loader.rs | 10 +- svm/tests/account_loader.rs | 214 ------------------------------- svm/tests/rent_state.rs | 90 ------------- 5 files changed, 39 insertions(+), 320 deletions(-) delete mode 100644 svm/tests/account_loader.rs delete mode 100644 svm/tests/rent_state.rs diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7ee51d5304922c..d1e22ff7162647 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4394,7 +4394,7 @@ impl Bank { pub fn unlock_accounts<'a>( &self, - txs_and_results: impl Iterator)>, + txs_and_results: impl Iterator)>, ) { self.rc.accounts.unlock_accounts(txs_and_results) } diff --git a/runtime/src/transaction_batch.rs b/runtime/src/transaction_batch.rs index 9ab4be21047682..5cbfbe5818ac33 100644 --- a/runtime/src/transaction_batch.rs +++ b/runtime/src/transaction_batch.rs @@ -100,7 +100,11 @@ mod tests { use { super::*, crate::genesis_utils::{create_genesis_config_with_leader, GenesisConfigInfo}, - solana_sdk::{signature::Keypair, system_transaction, transaction::{SanitizedTransaction, TransactionError}}, + solana_sdk::{ + signature::Keypair, + system_transaction, + transaction::{SanitizedTransaction, TransactionError}, + }, }; #[test] @@ -185,17 +189,36 @@ mod tests { let keypair2 = Keypair::new(); let pubkey2 = solana_sdk::pubkey::new_rand(); - let mut txs = vec![SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&mint_keypair, &pubkey, 1, genesis_config.hash()), - ).into()]; + let mut txs = + vec![ + SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( + &mint_keypair, + &pubkey, + 1, + genesis_config.hash(), + )) + .into(), + ]; if insert_conflicting_tx { - txs.push(SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&mint_keypair, &pubkey2, 1, genesis_config.hash()), - ).into()); + txs.push( + SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( + &mint_keypair, + &pubkey2, + 1, + genesis_config.hash(), + )) + .into(), + ); } - txs.push(SanitizedTransaction::from_transaction_for_tests( - system_transaction::transfer(&keypair2, &pubkey2, 1, genesis_config.hash()), - ).into()); + txs.push( + SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( + &keypair2, + &pubkey2, + 1, + genesis_config.hash(), + )) + .into(), + ); (bank, txs) } } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index a9ef7eb4db9d19..0e266d30f10d13 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -2069,7 +2069,7 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); let loaded_txs = load_accounts( &bank, - &[sanitized_tx.clone()], + &[sanitized_tx.clone().into()], &[(Ok(()), None, Some(0))], &mut error_counters, &FeeStructure::default(), @@ -2154,7 +2154,7 @@ mod tests { let results = load_accounts( &mock_bank, - &[sanitized_transaction], + &[sanitized_transaction.into()], &[lock_results], &mut error_counter, &FeeStructure::default(), @@ -2229,7 +2229,7 @@ mod tests { let result = load_accounts( &mock_bank, - &[sanitized_transaction.clone()], + &[sanitized_transaction.clone().into()], &[lock_results], &mut TransactionErrorMetrics::default(), &fee_structure, @@ -2248,7 +2248,7 @@ mod tests { let result = load_accounts( &mock_bank, - &[sanitized_transaction.clone()], + &[sanitized_transaction.clone().into()], &[lock_results.clone()], &mut TransactionErrorMetrics::default(), &fee_structure, @@ -2267,7 +2267,7 @@ mod tests { let result = load_accounts( &mock_bank, - &[sanitized_transaction.clone()], + &[sanitized_transaction.clone().into()], &[lock_results], &mut TransactionErrorMetrics::default(), &fee_structure, diff --git a/svm/tests/account_loader.rs b/svm/tests/account_loader.rs deleted file mode 100644 index f23f2def69f909..00000000000000 --- a/svm/tests/account_loader.rs +++ /dev/null @@ -1,214 +0,0 @@ -use { - crate::mock_bank::MockBankCallback, - solana_program_runtime::loaded_programs::LoadedProgramsForTxBatch, - solana_sdk::{ - account::{AccountSharedData, WritableAccount}, - fee::FeeStructure, - hash::Hash, - instruction::CompiledInstruction, - message::{LegacyMessage, Message, MessageHeader, SanitizedMessage}, - native_loader, - nonce_info::{NonceFull, NoncePartial}, - pubkey::Pubkey, - rent_collector::RENT_EXEMPT_RENT_EPOCH, - rent_debits::RentDebits, - signature::{Keypair, Signature, Signer}, - transaction::{SanitizedTransaction, TransactionError}, - }, - solana_svm::{ - account_loader::{load_accounts, LoadedTransaction, TransactionCheckResult}, - transaction_error_metrics::TransactionErrorMetrics, - }, - std::collections::HashMap, -}; - -mod mock_bank; - -#[test] -fn test_load_accounts_success() { - let key1 = Keypair::new(); - let key2 = Keypair::new(); - let key3 = Keypair::new(); - let key4 = Keypair::new(); - - let message = Message { - account_keys: vec![key2.pubkey(), key1.pubkey(), key4.pubkey()], - header: MessageHeader::default(), - instructions: vec![ - CompiledInstruction { - program_id_index: 1, - accounts: vec![0], - data: vec![], - }, - CompiledInstruction { - program_id_index: 1, - accounts: vec![2], - data: vec![], - }, - ], - recent_blockhash: Hash::default(), - }; - - let legacy = LegacyMessage::new(message); - let sanitized_message = SanitizedMessage::Legacy(legacy); - let mut mock_bank = MockBankCallback::default(); - let mut account_data = AccountSharedData::default(); - account_data.set_executable(true); - account_data.set_owner(key3.pubkey()); - mock_bank - .account_shared_data - .insert(key1.pubkey(), account_data); - - let mut account_data = AccountSharedData::default(); - account_data.set_lamports(200); - mock_bank - .account_shared_data - .insert(key2.pubkey(), account_data); - - let mut account_data = AccountSharedData::default(); - account_data.set_executable(true); - account_data.set_owner(native_loader::id()); - mock_bank - .account_shared_data - .insert(key3.pubkey(), account_data); - - let mut error_counter = TransactionErrorMetrics::default(); - let loaded_programs = LoadedProgramsForTxBatch::default(); - - let sanitized_transaction = SanitizedTransaction::new_for_tests( - sanitized_message, - vec![Signature::new_unique()], - false, - ); - let lock_results = - (Ok(()), Some(NoncePartial::default()), Some(20u64)) as TransactionCheckResult; - - let results = load_accounts( - &mock_bank, - &[sanitized_transaction.into()], - &[lock_results], - &mut error_counter, - &FeeStructure::default(), - None, - &HashMap::new(), - &loaded_programs, - ); - - let mut account_data = AccountSharedData::default(); - account_data.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); - - assert_eq!(results.len(), 1); - let (loaded_result, nonce) = results[0].clone(); - assert_eq!( - loaded_result.unwrap(), - LoadedTransaction { - accounts: vec![ - ( - key2.pubkey(), - mock_bank.account_shared_data[&key2.pubkey()].clone() - ), - ( - key1.pubkey(), - mock_bank.account_shared_data[&key1.pubkey()].clone() - ), - (key4.pubkey(), account_data), - ( - key3.pubkey(), - mock_bank.account_shared_data[&key3.pubkey()].clone() - ), - ], - program_indices: vec![vec![3, 1], vec![3, 1]], - rent: 0, - rent_debits: RentDebits::default() - } - ); - - assert_eq!( - nonce.unwrap(), - NonceFull::new( - Pubkey::from([0; 32]), - AccountSharedData::default(), - Some(mock_bank.account_shared_data[&key2.pubkey()].clone()) - ) - ); -} - -#[test] -fn test_load_accounts_error() { - let mock_bank = MockBankCallback::default(); - let message = Message { - account_keys: vec![Pubkey::new_from_array([0; 32])], - header: MessageHeader::default(), - instructions: vec![CompiledInstruction { - program_id_index: 0, - accounts: vec![], - data: vec![], - }], - recent_blockhash: Hash::default(), - }; - - let legacy = LegacyMessage::new(message); - let sanitized_message = SanitizedMessage::Legacy(legacy); - let sanitized_transaction = SanitizedTransaction::new_for_tests( - sanitized_message, - vec![Signature::new_unique()], - false, - ); - - let lock_results = (Ok(()), Some(NoncePartial::default()), None) as TransactionCheckResult; - let fee_structure = FeeStructure::default(); - - let result = load_accounts( - &mock_bank, - &[sanitized_transaction.clone().into()], - &[lock_results], - &mut TransactionErrorMetrics::default(), - &fee_structure, - None, - &HashMap::new(), - &LoadedProgramsForTxBatch::default(), - ); - - assert_eq!( - result, - vec![(Err(TransactionError::BlockhashNotFound), None)] - ); - - let lock_results = - (Ok(()), Some(NoncePartial::default()), Some(20u64)) as TransactionCheckResult; - - let result = load_accounts( - &mock_bank, - &[sanitized_transaction.clone().into()], - &[lock_results.clone()], - &mut TransactionErrorMetrics::default(), - &fee_structure, - None, - &HashMap::new(), - &LoadedProgramsForTxBatch::default(), - ); - - assert_eq!(result, vec![(Err(TransactionError::AccountNotFound), None)]); - - let lock_results = ( - Err(TransactionError::InvalidWritableAccount), - Some(NoncePartial::default()), - Some(20u64), - ) as TransactionCheckResult; - - let result = load_accounts( - &mock_bank, - &[sanitized_transaction.clone().into()], - &[lock_results], - &mut TransactionErrorMetrics::default(), - &fee_structure, - None, - &HashMap::new(), - &LoadedProgramsForTxBatch::default(), - ); - - assert_eq!( - result, - vec![(Err(TransactionError::InvalidWritableAccount), None)] - ); -} diff --git a/svm/tests/rent_state.rs b/svm/tests/rent_state.rs deleted file mode 100644 index d958f6f2f12c61..00000000000000 --- a/svm/tests/rent_state.rs +++ /dev/null @@ -1,90 +0,0 @@ -#![cfg(test)] - -use { - solana_program_runtime::{ - compute_budget::ComputeBudget, compute_budget_processor, - loaded_programs::LoadedProgramsForTxBatch, - }, - solana_sdk::{ - account::{AccountSharedData, WritableAccount}, - fee::FeeStructure, - hash::Hash, - native_loader, - native_token::sol_to_lamports, - pubkey::Pubkey, - rent::Rent, - signature::{Keypair, Signer}, - system_transaction, - transaction::SanitizedTransaction, - transaction_context::TransactionContext, - }, - solana_svm::{ - account_loader::load_accounts, transaction_account_state_info::TransactionAccountStateInfo, - transaction_error_metrics::TransactionErrorMetrics, - }, - std::collections::HashMap, -}; - -mod mock_bank; - -#[test] -fn test_rent_state_list_len() { - let mint_keypair = Keypair::new(); - let mut bank = mock_bank::MockBankCallback::default(); - let recipient = Pubkey::new_unique(); - let last_block_hash = Hash::new_unique(); - - let mut system_data = AccountSharedData::default(); - system_data.set_executable(true); - system_data.set_owner(native_loader::id()); - bank.account_shared_data - .insert(Pubkey::new_from_array([0u8; 32]), system_data); - - let mut mint_data = AccountSharedData::default(); - mint_data.set_lamports(2); - bank.account_shared_data - .insert(mint_keypair.pubkey(), mint_data); - - bank.account_shared_data - .insert(recipient, AccountSharedData::default()); - - let tx = system_transaction::transfer( - &mint_keypair, - &recipient, - sol_to_lamports(1.), - last_block_hash, - ); - let num_accounts = tx.message().account_keys.len(); - let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); - let mut error_counters = TransactionErrorMetrics::default(); - let loaded_txs = load_accounts( - &bank, - &[sanitized_tx.clone().into()], - &[(Ok(()), None, Some(0))], - &mut error_counters, - &FeeStructure::default(), - None, - &HashMap::new(), - &LoadedProgramsForTxBatch::default(), - ); - - let compute_budget = ComputeBudget::new(u64::from( - compute_budget_processor::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, - )); - let transaction_context = TransactionContext::new( - loaded_txs[0].0.as_ref().unwrap().accounts.clone(), - Rent::default(), - compute_budget.max_invoke_stack_height, - compute_budget.max_instruction_trace_length, - ); - - assert_eq!( - TransactionAccountStateInfo::new( - &Rent::default(), - &transaction_context, - sanitized_tx.message() - ) - .len(), - num_accounts, - ); -} From 8c168a59d1efe4716d41e3971da86071f007d7b0 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Mon, 11 Mar 2024 22:10:56 -0700 Subject: [PATCH 22/42] Use Arc, metrics: Arc, - perf_track_metrics: histogram::Histogram, + perf_track_metrics: Arc>, } #[allow(dead_code)] @@ -52,7 +52,7 @@ impl ConsumeWorker { consumed_sender, leader_bank_notifier, metrics: Arc::new(ConsumeWorkerMetrics::new(id)), - perf_track_metrics: histogram::Histogram::default(), + perf_track_metrics: Arc::new(Mutex::new(histogram::Histogram::default())), } } @@ -98,7 +98,7 @@ impl ConsumeWorker { bank, &work.transactions, &work.max_age_slots, - Some(&mut self.perf_track_metrics), + Some(&mut self.perf_track_metrics.lock().unwrap()), ); self.metrics.update_for_consume(&output); From 210807f762a47dc229320abb0ea15ce4020ab867 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Mon, 11 Mar 2024 22:38:12 -0700 Subject: [PATCH 23/42] runtime-transaction/src/extended_transaction.rs --- runtime-transaction/src/extended_transaction.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 runtime-transaction/src/extended_transaction.rs diff --git a/runtime-transaction/src/extended_transaction.rs b/runtime-transaction/src/extended_transaction.rs new file mode 100644 index 00000000000000..13ca194d13dc32 --- /dev/null +++ b/runtime-transaction/src/extended_transaction.rs @@ -0,0 +1,17 @@ +use {solana_sdk::transaction::SanitizedTransaction, std::time::Instant}; + +/// Sanitized transaction with optional start_time +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct ExtendedSanitizedTransaction { + pub transaction: SanitizedTransaction, + pub start_time: Option, +} + +impl From for ExtendedSanitizedTransaction { + fn from(value: SanitizedTransaction) -> Self { + Self { + transaction: value, + start_time: None, + } + } +} From ee0d4be8da7190ac26a2b81964078405e03e3ba5 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Mon, 11 Mar 2024 22:49:17 -0700 Subject: [PATCH 24/42] Removed mut on ConsumeWorker --- core/src/banking_stage/consume_worker.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 6fbe81f22f7f3c..739875a6f7e4ed 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -60,23 +60,19 @@ impl ConsumeWorker { self.metrics.clone() } - pub fn run(mut self) -> Result<(), ConsumeWorkerError> { + pub fn run(self) -> Result<(), ConsumeWorkerError> { loop { let work = self.consume_receiver.recv()?; self.consume_loop(work)?; } } - fn consume_loop(&mut self, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { + fn consume_loop(&self, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { let Some(mut bank) = self.get_consume_bank() else { return self.retry_drain(work); }; - // We need to have a mutable borrow in consume, so move the receiver out - let (_, dummy_receiver) = crossbeam_channel::unbounded(); - let consume_receiver = std::mem::replace(&mut self.consume_receiver, dummy_receiver); - - for work in try_drain_iter(work, &consume_receiver) { + for work in try_drain_iter(work, &self.consume_receiver) { if bank.is_complete() { if let Some(new_bank) = self.get_consume_bank() { bank = new_bank; @@ -87,13 +83,11 @@ impl ConsumeWorker { self.consume(&bank, work)?; } - // restore it - self.consume_receiver = consume_receiver; Ok(()) } /// Consume a single batch. - fn consume(&mut self, bank: &Arc, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { + fn consume(&self, bank: &Arc, work: ConsumeWork) -> Result<(), ConsumeWorkerError> { let output = self.consumer.process_and_record_aged_transactions( bank, &work.transactions, From d28e4e62336f7f2c218cfb937df62c771582554f Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 00:05:14 -0700 Subject: [PATCH 25/42] report tracked packet perf with central scheduler --- core/src/banking_stage/consume_worker.rs | 42 ++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 739875a6f7e4ed..21f52299cde68e 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -34,7 +34,6 @@ pub(crate) struct ConsumeWorker { leader_bank_notifier: Arc, metrics: Arc, - perf_track_metrics: Arc>, } #[allow(dead_code)] @@ -52,7 +51,6 @@ impl ConsumeWorker { consumed_sender, leader_bank_notifier, metrics: Arc::new(ConsumeWorkerMetrics::new(id)), - perf_track_metrics: Arc::new(Mutex::new(histogram::Histogram::default())), } } @@ -92,7 +90,7 @@ impl ConsumeWorker { bank, &work.transactions, &work.max_age_slots, - Some(&mut self.perf_track_metrics.lock().unwrap()), + Some(&mut self.metrics.perf_track_metrics.lock().unwrap()), ); self.metrics.update_for_consume(&output); @@ -161,6 +159,7 @@ pub(crate) struct ConsumeWorkerMetrics { count_metrics: ConsumeWorkerCountMetrics, error_metrics: ConsumeWorkerTransactionErrorMetrics, timing_metrics: ConsumeWorkerTimingMetrics, + perf_track_metrics: Arc>, } impl ConsumeWorkerMetrics { @@ -173,9 +172,45 @@ impl ConsumeWorkerMetrics { self.count_metrics.report_and_reset(self.id); self.timing_metrics.report_and_reset(self.id); self.error_metrics.report_and_reset(self.id); + self.report_and_reset_perf_track_metrics(self.id); } } + fn report_and_reset_perf_track_metrics(&self, id: u32) { + let process_sampled_packets_us_hist = { + let mut metrics = self.perf_track_metrics.lock().unwrap(); + let local_metrics = metrics.clone(); + metrics.clear(); + local_metrics + }; + datapoint_info!( + "banking_stage_tracked_packet_metrics", + ("id", id, i64), + ( + "process_sampled_packets_us_90pct", + process_sampled_packets_us_hist + .percentile(90.0) + .unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_min", + process_sampled_packets_us_hist.minimum().unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_max", + process_sampled_packets_us_hist.maximum().unwrap_or(0), + i64 + ), + ( + "process_sampled_packets_us_mean", + process_sampled_packets_us_hist.mean().unwrap_or(0), + i64 + ), + ); + } + fn new(id: u32) -> Self { Self { id, @@ -184,6 +219,7 @@ impl ConsumeWorkerMetrics { count_metrics: ConsumeWorkerCountMetrics::default(), error_metrics: ConsumeWorkerTransactionErrorMetrics::default(), timing_metrics: ConsumeWorkerTimingMetrics::default(), + perf_track_metrics: Arc::new(Mutex::new(histogram::Histogram::default())), } } From 4a20ac32c665fcd28a3c82bd51ab39470f771af5 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 00:15:04 -0700 Subject: [PATCH 26/42] report count of sampled packets in the report period --- core/src/banking_stage/consume_worker.rs | 5 +++++ .../banking_stage/leader_slot_timing_metrics.rs | 5 +++++ core/src/sigverify_stage.rs | 5 +++++ streamer/src/quic.rs | 15 ++++++++++++--- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 21f52299cde68e..1b0dad208fefc7 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -208,6 +208,11 @@ impl ConsumeWorkerMetrics { process_sampled_packets_us_hist.mean().unwrap_or(0), i64 ), + ( + "process_sampled_packets_count", + process_sampled_packets_us_hist.entries(), + i64 + ), ); } diff --git a/core/src/banking_stage/leader_slot_timing_metrics.rs b/core/src/banking_stage/leader_slot_timing_metrics.rs index 34ce64b31c34f3..a2cdfa33bd8d43 100644 --- a/core/src/banking_stage/leader_slot_timing_metrics.rs +++ b/core/src/banking_stage/leader_slot_timing_metrics.rs @@ -289,6 +289,11 @@ impl ProcessPacketsTimings { self.process_sampled_packets_us_hist.mean().unwrap_or(0), i64 ), + ( + "process_sampled_packets_count", + self.process_sampled_packets_us_hist.entries(), + i64 + ), ); } } diff --git a/core/src/sigverify_stage.rs b/core/src/sigverify_stage.rs index aa9f4b7c4a2869..4da3e6c874e767 100644 --- a/core/src/sigverify_stage.rs +++ b/core/src/sigverify_stage.rs @@ -206,6 +206,11 @@ impl SigVerifierStats { self.process_sampled_packets_us_hist.mean().unwrap_or(0), i64 ), + ( + "process_sampled_packets_count", + self.process_sampled_packets_us_hist.entries(), + i64 + ), ( "batches_90pct", self.batches_hist.percentile(90.0).unwrap_or(0), diff --git a/streamer/src/quic.rs b/streamer/src/quic.rs index a8d4cd01291cd2..3b1b6b21adf468 100644 --- a/streamer/src/quic.rs +++ b/streamer/src/quic.rs @@ -181,8 +181,13 @@ pub struct StreamStats { impl StreamStats { pub fn report(&self, name: &'static str) { - let mut process_sampled_packets_us_hist = - self.process_sampled_packets_us_hist.lock().unwrap(); + let process_sampled_packets_us_hist = { + let mut metrics = self.process_sampled_packets_us_hist.lock().unwrap(); + let process_sampled_packets_us_hist = metrics.clone(); + metrics.clear(); + process_sampled_packets_us_hist + }; + datapoint_info!( name, ( @@ -451,13 +456,17 @@ impl StreamStats { process_sampled_packets_us_hist.mean().unwrap_or(0), i64 ), + ( + "process_sampled_packets_count", + process_sampled_packets_us_hist.entries(), + i64 + ), ( "perf_track_overhead_us", self.perf_track_overhead_us.swap(0, Ordering::Relaxed), i64 ), ); - process_sampled_packets_us_hist.clear(); } } From 770deec7133c35adc8a7764641f7ac066fec1691 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 10:23:05 -0700 Subject: [PATCH 27/42] fixed test failures --- entry/benches/entry_sigverify.rs | 4 +++- svm/src/transaction_processor.rs | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/entry/benches/entry_sigverify.rs b/entry/benches/entry_sigverify.rs index b3a1b7b5cdb3e6..5a1c871319ca5c 100644 --- a/entry/benches/entry_sigverify.rs +++ b/entry/benches/entry_sigverify.rs @@ -3,6 +3,7 @@ extern crate test; use { solana_entry::entry::{self, VerifyRecyclers}, solana_perf::test_tx::test_tx, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ hash::Hash, transaction::{ @@ -26,7 +27,7 @@ fn bench_gpusigverify(bencher: &mut Bencher) { let verify_transaction = { move |versioned_tx: VersionedTransaction, verification_mode: TransactionVerificationMode| - -> Result { + -> Result { let sanitized_tx = { let message_hash = if verification_mode == TransactionVerificationMode::FullVerification { @@ -41,6 +42,7 @@ fn bench_gpusigverify(bencher: &mut Bencher) { None, SimpleAddressLoader::Disabled, ) + .map(|t| t.into()) }?; Ok(sanitized_tx) diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 4b0ca0c4194899..294241159628e6 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1935,10 +1935,10 @@ mod tests { ); let transactions = vec![ - sanitized_transaction_1.clone(), - sanitized_transaction_2.clone(), - sanitized_transaction_2, - sanitized_transaction_1, + sanitized_transaction_1.clone().into(), + sanitized_transaction_2.clone().into(), + sanitized_transaction_2.into(), + sanitized_transaction_1.into(), ]; let mut lock_results = vec![ (Ok(()), None, Some(25)), From c19b5f30ff0f95dd88aec05fc3c8f7127759eeb0 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 12:41:18 -0700 Subject: [PATCH 28/42] missing Cargo.lock --- programs/sbf/Cargo.lock | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 51fa9e165efc2e..966dbad946b633 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -4666,6 +4666,7 @@ dependencies = [ "solana-nohash-hasher", "solana-program-runtime", "solana-rayon-threadlimit", + "solana-runtime-transaction", "solana-sdk", "solana-stake-program", "solana-svm", @@ -4732,6 +4733,7 @@ dependencies = [ "solana-banks-interface", "solana-client", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-send-transaction-service", "solana-svm", @@ -4977,6 +4979,7 @@ dependencies = [ "solana-rpc", "solana-rpc-client-api", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-send-transaction-service", "solana-streamer", @@ -5051,6 +5054,7 @@ dependencies = [ "solana-metrics", "solana-perf", "solana-rayon-threadlimit", + "solana-runtime-transaction", "solana-sdk", ] @@ -5238,6 +5242,7 @@ dependencies = [ "solana-program-runtime", "solana-rayon-threadlimit", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-stake-program", "solana-storage-bigtable", @@ -5593,6 +5598,7 @@ dependencies = [ "solana-rayon-threadlimit", "solana-rpc-client-api", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-send-transaction-service", "solana-stake-program", @@ -5685,6 +5691,7 @@ dependencies = [ "dir-diff", "flate2", "fnv", + "histogram", "im", "index_list", "itertools", @@ -5723,6 +5730,7 @@ dependencies = [ "solana-perf", "solana-program-runtime", "solana-rayon-threadlimit", + "solana-runtime-transaction", "solana-sdk", "solana-stake-program", "solana-svm", @@ -5742,6 +5750,17 @@ dependencies = [ "zstd", ] +[[package]] +name = "solana-runtime-transaction" +version = "2.0.0" +dependencies = [ + "log", + "rustc_version", + "solana-program-runtime", + "solana-sdk", + "thiserror", +] + [[package]] name = "solana-sbf-programs" version = "2.0.0" @@ -6343,6 +6362,7 @@ dependencies = [ name = "solana-svm" version = "2.0.0" dependencies = [ + "histogram", "itertools", "log", "percentage", @@ -6354,6 +6374,7 @@ dependencies = [ "solana-measure", "solana-metrics", "solana-program-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-system-program", ] @@ -6437,7 +6458,7 @@ dependencies = [ [[package]] name = "solana-transaction-metrics-tracker" -version = "1.19.0" +version = "2.0.0" dependencies = [ "Inflector", "base64 0.21.7", @@ -6523,6 +6544,7 @@ dependencies = [ name = "solana-unified-scheduler-logic" version = "2.0.0" dependencies = [ + "solana-runtime-transaction", "solana-sdk", ] @@ -6537,6 +6559,7 @@ dependencies = [ "solana-ledger", "solana-program-runtime", "solana-runtime", + "solana-runtime-transaction", "solana-sdk", "solana-unified-scheduler-logic", "solana-vote", From 9cb75ec36bd414c5e4b18793d2ce55a8e81b630e Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 13:06:12 -0700 Subject: [PATCH 29/42] clippy issue --- entry/benches/entry_sigverify.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/entry/benches/entry_sigverify.rs b/entry/benches/entry_sigverify.rs index 5a1c871319ca5c..540a745107fade 100644 --- a/entry/benches/entry_sigverify.rs +++ b/entry/benches/entry_sigverify.rs @@ -75,7 +75,7 @@ fn bench_cpusigverify(bencher: &mut Bencher) { .collect::>(); let verify_transaction = { - move |versioned_tx: VersionedTransaction| -> Result { + move |versioned_tx: VersionedTransaction| -> Result { let sanitized_tx = { let message_hash = versioned_tx.verify_and_hash_message()?; SanitizedTransaction::try_create( @@ -84,7 +84,7 @@ fn bench_cpusigverify(bencher: &mut Bencher) { None, SimpleAddressLoader::Disabled, ) - }?; + }?.into(); Ok(sanitized_tx) } From 93861d9961f1636dd9bb81c5d12cfc394f9c03f0 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 14:18:52 -0700 Subject: [PATCH 30/42] clippy issue --- entry/benches/entry_sigverify.rs | 3 ++- runtime/benches/prioritization_fee_cache.rs | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/entry/benches/entry_sigverify.rs b/entry/benches/entry_sigverify.rs index 540a745107fade..4d9f89d949efd0 100644 --- a/entry/benches/entry_sigverify.rs +++ b/entry/benches/entry_sigverify.rs @@ -84,7 +84,8 @@ fn bench_cpusigverify(bencher: &mut Bencher) { None, SimpleAddressLoader::Disabled, ) - }?.into(); + }? + .into(); Ok(sanitized_tx) } diff --git a/runtime/benches/prioritization_fee_cache.rs b/runtime/benches/prioritization_fee_cache.rs index 8c6bf1fe0a7d68..5913a76a14581c 100644 --- a/runtime/benches/prioritization_fee_cache.rs +++ b/runtime/benches/prioritization_fee_cache.rs @@ -55,6 +55,7 @@ fn bench_process_transactions_single_slot(bencher: &mut Bencher) { &Pubkey::new_unique(), &Pubkey::new_unique(), ) + .into() }) .collect(); @@ -82,6 +83,7 @@ fn process_transactions_multiple_slots(banks: &[Arc], num_slots: usize, nu &Pubkey::new_unique(), &Pubkey::new_unique(), ) + .into() }) .collect(); From 6b38b513b010bd16c18876dec71eebd3b99e7f5c Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 14:28:46 -0700 Subject: [PATCH 31/42] clippy issue --- ledger/benches/blockstore_processor.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ledger/benches/blockstore_processor.rs b/ledger/benches/blockstore_processor.rs index f1c335596140b0..3772ef7cf1306c 100644 --- a/ledger/benches/blockstore_processor.rs +++ b/ledger/benches/blockstore_processor.rs @@ -15,6 +15,7 @@ use { bank::Bank, prioritization_fee_cache::PrioritizationFeeCache, transaction_batch::TransactionBatch, }, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::Account, feature_set::apply_cost_tracker_during_replay, signature::Keypair, signer::Signer, stake_history::Epoch, system_program, system_transaction, @@ -53,7 +54,7 @@ fn create_funded_accounts(bank: &Bank, num: usize) -> Vec { accounts } -fn create_transactions(bank: &Bank, num: usize) -> Vec { +fn create_transactions(bank: &Bank, num: usize) -> Vec { let funded_accounts = create_funded_accounts(bank, 2 * num); funded_accounts .into_par_iter() @@ -63,7 +64,7 @@ fn create_transactions(bank: &Bank, num: usize) -> Vec { let to = &chunk[1]; system_transaction::transfer(from, &to.pubkey(), 1, bank.last_blockhash()) }) - .map(SanitizedTransaction::from_transaction_for_tests) + .map(|t| SanitizedTransaction::from_transaction_for_tests(t).into()) .collect() } From b2c197318716c2332df5368dd05bdbcc0b64f755 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 15:00:57 -0700 Subject: [PATCH 32/42] clippy issue --- core/benches/consumer.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/benches/consumer.rs b/core/benches/consumer.rs index f056fdd0d49a34..10027558397920 100644 --- a/core/benches/consumer.rs +++ b/core/benches/consumer.rs @@ -20,6 +20,7 @@ use { poh_service::PohService, }, solana_runtime::bank::Bank, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ account::Account, feature_set::apply_cost_tracker_during_replay, signature::Keypair, signer::Signer, stake_history::Epoch, system_program, system_transaction, @@ -62,7 +63,7 @@ fn create_funded_accounts(bank: &Bank, num: usize) -> Vec { accounts } -fn create_transactions(bank: &Bank, num: usize) -> Vec { +fn create_transactions(bank: &Bank, num: usize) -> Vec { let funded_accounts = create_funded_accounts(bank, 2 * num); funded_accounts .into_par_iter() @@ -72,7 +73,7 @@ fn create_transactions(bank: &Bank, num: usize) -> Vec { let to = &chunk[1]; system_transaction::transfer(from, &to.pubkey(), 1, bank.last_blockhash()) }) - .map(SanitizedTransaction::from_transaction_for_tests) + .map(|t| SanitizedTransaction::from_transaction_for_tests(t).into()) .collect() } @@ -167,6 +168,7 @@ fn bench_process_and_record_transactions( &bank, transaction_iter.next().unwrap(), 0, + &mut None, ); assert!(summary .execute_and_commit_transactions_output From 899a83798c9f80d234e292b3f69016a48ed336f8 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 15:14:23 -0700 Subject: [PATCH 33/42] clippy issue --- programs/sbf/tests/programs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 22969bc482a28e..4cb85d3a0ebe73 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -112,6 +112,7 @@ fn process_transaction_and_record_inner( }, &mut ExecuteTimings::default(), None, + None, ) .0; let result = results @@ -676,7 +677,7 @@ fn test_return_data_and_log_data_syscall() { let blockhash = bank.last_blockhash(); let message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); let transaction = Transaction::new(&[&mint_keypair], message, blockhash); - let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(transaction); + let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(transaction).into(); let result = bank.simulate_transaction(&sanitized_tx, false); From f3bd6b75e7ad441c6b24426904baa656991f568e Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 15:31:09 -0700 Subject: [PATCH 34/42] clippy issue --- programs/sbf/tests/programs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 4cb85d3a0ebe73..9e8eef6be01b2f 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -159,6 +159,7 @@ fn execute_transactions( ExecutionRecordingConfig::new_single_setting(true), &mut timings, None, + None, ); let tx_post_token_balances = collect_token_balances(&bank, &batch, &mut mint_decimals); From 62a376fab51c077ad0f973aef4676b186a8434a3 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 17:05:05 -0700 Subject: [PATCH 35/42] Use wrappers in ExtendedSanitizedTransaction to simplify changes --- accounts-db/src/accounts.rs | 8 +- accounts-db/src/accounts_db.rs | 2 +- core/src/banking_stage/consumer.rs | 11 ++- .../forward_packet_batches_by_accounts.rs | 2 +- core/src/banking_stage/qos_service.rs | 4 +- .../prio_graph_scheduler.rs | 17 ++-- .../scheduler_controller.rs | 26 ++---- entry/src/entry.rs | 2 +- ledger/src/blockstore.rs | 2 +- ledger/src/blockstore_processor.rs | 4 +- ledger/src/token_balances.rs | 6 +- rpc/src/rpc.rs | 4 +- rpc/src/transaction_status_service.rs | 25 ++---- .../src/extended_transaction.rs | 83 ++++++++++++++++++- runtime/src/bank.rs | 51 +++++------- runtime/src/bank_utils.rs | 5 +- runtime/src/compute_budget_details.rs | 11 +++ runtime/src/prioritization_fee_cache.rs | 7 +- svm/src/account_loader.rs | 2 +- svm/src/transaction_processor.rs | 7 +- 20 files changed, 166 insertions(+), 113 deletions(-) diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 52674ea37824eb..3da2dbe1bea59a 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -579,7 +579,7 @@ impl Accounts { tx_account_lock_limit: usize, ) -> Vec> { let tx_account_locks_results: Vec> = txs - .map(|tx| tx.transaction.get_account_locks(tx_account_lock_limit)) + .map(|tx| tx.get_account_locks(tx_account_lock_limit)) .collect(); self.lock_accounts_inner(tx_account_locks_results) } @@ -595,7 +595,7 @@ impl Accounts { let tx_account_locks_results: Vec> = txs .zip(results) .map(|(tx, result)| match result { - Ok(()) => tx.transaction.get_account_locks(tx_account_lock_limit), + Ok(()) => tx.get_account_locks(tx_account_lock_limit), Err(err) => Err(err), }) .collect(); @@ -629,7 +629,7 @@ impl Accounts { ) { let keys: Vec<_> = txs_and_results .filter(|(_, res)| res.is_ok()) - .map(|(tx, _)| tx.transaction.get_account_locks_unchecked()) + .map(|(tx, _)| tx.get_account_locks_unchecked()) .collect(); if keys.is_empty() { return; @@ -710,7 +710,7 @@ impl Accounts { } }; - let message = tx.transaction.message(); + let message = tx.message(); let loaded_transaction = tx_load_result.as_mut().unwrap(); let mut fee_payer_index = None; for (i, (address, account)) in (0..message.account_keys().len()) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 5a0a6ea32c95ac..628bbb1432dd2e 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -6391,7 +6391,7 @@ impl AccountsDb { self.notify_account_at_accounts_update( slot, &account, - &txn.map(|txn| &txn.transaction), + &txn.map(|txn| txn.transaction()), accounts_and_meta_to_store.pubkey(i), &mut write_version_producer, ); diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index fcd428582c4957..cf7b2c936a42f0 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -455,7 +455,7 @@ impl Consumer { // Re-sanitized transaction should be equal to the original transaction, // but whether it will pass sanitization needs to be checked. let resanitized_tx = - bank.fully_verify_transaction(tx.transaction.to_versioned_transaction())?; + bank.fully_verify_transaction(tx.to_versioned_transaction())?; if resanitized_tx != *tx { // Sanitization before/after epoch give different transaction data - do not execute. return Err(TransactionError::ResanitizationNeeded); @@ -463,7 +463,7 @@ impl Consumer { } else { // 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.transaction.message().message_address_table_lookups(); + let lookup_tables = tx.message().message_address_table_lookups(); if !lookup_tables.is_empty() { bank.load_addresses(lookup_tables)?; } @@ -600,7 +600,6 @@ impl Consumer { .filter_map(|transaction| { let round_compute_unit_price_enabled = false; // TODO get from working_bank.feature_set transaction - .transaction .get_compute_budget_details(round_compute_unit_price_enabled) .map(|details| details.compute_unit_price) }) @@ -640,7 +639,7 @@ impl Consumer { .zip(batch.sanitized_transactions()) .filter_map(|(execution_result, tx)| { if execution_result.was_executed() { - Some(tx.transaction.to_versioned_transaction()) + Some(tx.to_versioned_transaction()) } else { None } @@ -1576,7 +1575,7 @@ mod tests { }; let mut cost = - CostModel::calculate_cost(&transactions[0].transaction, &bank.feature_set); + CostModel::calculate_cost(transactions[0].transaction(), &bank.feature_set); if let TransactionCost::Transaction(ref mut usage_cost) = cost { usage_cost.programs_execution_cost = actual_programs_execution_cost; } @@ -1584,7 +1583,7 @@ mod tests { block_cost + cost.sum() } else { block_cost - + CostModel::calculate_cost(&transactions[0].transaction, &bank.feature_set) + + CostModel::calculate_cost(transactions[0].transaction(), &bank.feature_set) .sum() }; diff --git a/core/src/banking_stage/forward_packet_batches_by_accounts.rs b/core/src/banking_stage/forward_packet_batches_by_accounts.rs index 3db9797efcad8b..5f1f39291b59c5 100644 --- a/core/src/banking_stage/forward_packet_batches_by_accounts.rs +++ b/core/src/banking_stage/forward_packet_batches_by_accounts.rs @@ -63,7 +63,7 @@ impl ForwardBatch { immutable_packet: Arc, feature_set: &FeatureSet, ) -> Result { - let tx_cost = CostModel::calculate_cost(&sanitized_transaction.transaction, feature_set); + let tx_cost = CostModel::calculate_cost(sanitized_transaction.transaction(), feature_set); let res = self.cost_tracker.try_add(&tx_cost); if res.is_ok() { self.forwardable_packets.push(immutable_packet); diff --git a/core/src/banking_stage/qos_service.rs b/core/src/banking_stage/qos_service.rs index 78cd5bfd8109c8..08dc7a460788b1 100644 --- a/core/src/banking_stage/qos_service.rs +++ b/core/src/banking_stage/qos_service.rs @@ -75,7 +75,7 @@ impl QosService { let txs_costs: Vec<_> = transactions .zip(pre_results) .map(|(tx, pre_result)| { - pre_result.map(|()| CostModel::calculate_cost(&tx.transaction, feature_set)) + pre_result.map(|()| CostModel::calculate_cost(tx.transaction(), feature_set)) }) .collect(); compute_cost_time.stop(); @@ -632,7 +632,7 @@ mod tests { .map(|(index, cost)| { assert_eq!( cost.as_ref().unwrap().sum(), - CostModel::calculate_cost(&txs[index].transaction, &FeatureSet::all_enabled()) + CostModel::calculate_cost(txs[index].transaction(), &FeatureSet::all_enabled()) .sum() ); }) 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 59bc13c6fe29a6..5a585dd79fd9b0 100644 --- a/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs +++ b/core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs @@ -159,15 +159,15 @@ impl PrioGraphScheduler { } // Check if this transaction conflicts with any blocked transactions - if !blocking_locks.check_locks(transaction.transaction.message()) { - blocking_locks.take_locks(transaction.transaction.message()); + if !blocking_locks.check_locks(transaction.message()) { + blocking_locks.take_locks(transaction.message()); unschedulable_ids.push(id); saturating_add_assign!(num_unschedulable, 1); continue; } // Schedule the transaction if it can be. - let transaction_locks = transaction.transaction.get_account_locks_unchecked(); + let transaction_locks = transaction.get_account_locks_unchecked(); let Some(thread_id) = self.account_locks.try_lock_accounts( transaction_locks.writable.into_iter(), transaction_locks.readonly.into_iter(), @@ -180,7 +180,7 @@ impl PrioGraphScheduler { ) }, ) else { - blocking_locks.take_locks(transaction.transaction.message()); + blocking_locks.take_locks(transaction.message()); unschedulable_ids.push(id); saturating_add_assign!(num_unschedulable, 1); continue; @@ -331,7 +331,7 @@ impl PrioGraphScheduler { ) { let thread_id = self.in_flight_tracker.complete_batch(batch_id); for transaction in transactions { - let account_locks = transaction.transaction.get_account_locks_unchecked(); + let account_locks = transaction.get_account_locks_unchecked(); self.account_locks.unlock_accounts( account_locks.writable.into_iter(), account_locks.readonly.into_iter(), @@ -410,7 +410,7 @@ impl PrioGraphScheduler { fn get_transaction_account_access( transaction: &SanitizedTransactionTTL, ) -> impl Iterator + '_ { - let message = transaction.transaction.transaction.message(); + let message = transaction.transaction.message(); message .account_keys() .iter() @@ -782,9 +782,8 @@ mod tests { ]); // 2nd transaction should be filtered out and dropped before locking. - let pre_lock_filter = |tx: &ExtendedSanitizedTransaction| { - tx.transaction.message().fee_payer() != &keypair.pubkey() - }; + let pre_lock_filter = + |tx: &ExtendedSanitizedTransaction| tx.message().fee_payer() != &keypair.pubkey(); let scheduling_summary = scheduler .schedule(&mut container, test_pre_graph_filter, pre_lock_filter) .unwrap(); diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index b5a7a786424ec9..5206e2a5a674a5 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -209,11 +209,7 @@ impl SchedulerController { .zip(transactions) .map(|((result, _nonce, _lamports), tx)| { result?; // if there's already error do nothing - Consumer::check_fee_payer_unlocked( - bank, - tx.transaction.message(), - &mut error_counters, - ) + Consumer::check_fee_payer_unlocked(bank, tx.message(), &mut error_counters) }) .collect(); @@ -496,9 +492,9 @@ impl SchedulerController { fee_budget_limits: &FeeBudgetLimits, bank: &Bank, ) -> (u64, u64) { - let cost = CostModel::calculate_cost(&transaction.transaction, &bank.feature_set).sum(); + let cost = CostModel::calculate_cost(transaction.transaction(), &bank.feature_set).sum(); let fee = bank.fee_structure.calculate_fee( - transaction.transaction.message(), + transaction.message(), 5_000, // this just needs to be non-zero fee_budget_limits, bank.feature_set @@ -749,7 +745,7 @@ mod tests { let message_hashes = consume_work .transactions .iter() - .map(|tx| tx.transaction.message_hash()) + .map(|tx| tx.message_hash()) .collect_vec(); assert_eq!(message_hashes, vec![&tx2_hash, &tx1_hash]); } @@ -807,11 +803,7 @@ mod tests { let num_txs_per_batch = consume_works.iter().map(|cw| cw.ids.len()).collect_vec(); let message_hashes = consume_works .iter() - .flat_map(|cw| { - cw.transactions - .iter() - .map(|tx| tx.transaction.message_hash()) - }) + .flat_map(|cw| cw.transactions.iter().map(|tx| tx.message_hash())) .collect_vec(); assert_eq!(num_txs_per_batch, vec![1; 2]); assert_eq!(message_hashes, vec![&tx2_hash, &tx1_hash]); @@ -934,14 +926,14 @@ mod tests { .unwrap() .transactions .iter() - .map(|tx| *tx.transaction.message_hash()) + .map(|tx| *tx.message_hash()) .collect_vec(); let t1_actual = consume_work_receivers[1] .try_recv() .unwrap() .transactions .iter() - .map(|tx| *tx.transaction.message_hash()) + .map(|tx| *tx.message_hash()) .collect_vec(); assert_eq!(t0_actual, t0_expected); @@ -1000,7 +992,7 @@ mod tests { let message_hashes = consume_work .transactions .iter() - .map(|tx| tx.transaction.message_hash()) + .map(|tx| tx.message_hash()) .collect_vec(); assert_eq!(message_hashes, vec![&tx2_hash, &tx1_hash]); @@ -1020,7 +1012,7 @@ mod tests { let message_hashes = consume_work .transactions .iter() - .map(|tx| tx.transaction.message_hash()) + .map(|tx| tx.message_hash()) .collect_vec(); assert_eq!(message_hashes, vec![&tx1_hash]); } diff --git a/entry/src/entry.rs b/entry/src/entry.rs index 2a279408324d83..4f6b36e767256d 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -562,7 +562,7 @@ fn start_verify_transactions_gpu( } let entry_tx_iter = slice .into_par_iter() - .map(|tx| tx.transaction.to_versioned_transaction()); + .map(|tx| tx.to_versioned_transaction()); let res = packet_batch .par_iter_mut() diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 24f94f762b137a..f15976abdb241b 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -3095,7 +3095,7 @@ impl Blockstore { // Attempt to verify transaction and load addresses from the current bank, // or manually scan the transaction for addresses if the transaction. if let Ok(tx) = bank.fully_verify_transaction(tx.clone()) { - add_to_set(&result, tx.transaction.message().account_keys().iter()); + add_to_set(&result, tx.message().account_keys().iter()); } else { add_to_set(&result, tx.message.static_account_keys()); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 3182fb9d2ea635..32001944ec10b4 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -120,7 +120,7 @@ fn get_first_error( { if let Err(ref err) = result { if first_err.is_none() { - first_err = Some((result.clone(), *transaction.transaction.signature())); + first_err = Some((result.clone(), *transaction.signature())); } warn!( "Unexpected validator error: {:?}, transaction: {:?}", @@ -428,7 +428,7 @@ fn rebatch_and_execute_batches( let tx_costs = sanitized_txs .iter() .map(|tx| { - let tx_cost = CostModel::calculate_cost(&tx.transaction, &bank.feature_set); + let tx_cost = CostModel::calculate_cost(tx.transaction(), &bank.feature_set); let cost = tx_cost.sum(); minimal_tx_cost = std::cmp::min(minimal_tx_cost, cost); total_cost = total_cost.saturating_add(cost); diff --git a/ledger/src/token_balances.rs b/ledger/src/token_balances.rs index ff133127a14b4a..204bd4335972aa 100644 --- a/ledger/src/token_balances.rs +++ b/ledger/src/token_balances.rs @@ -43,15 +43,13 @@ pub fn collect_token_balances( let mut collect_time = Measure::start("collect_token_balances"); for transaction in batch.sanitized_transactions() { - let account_keys = transaction.transaction.message().account_keys(); + let account_keys = transaction.message().account_keys(); let has_token_program = account_keys.iter().any(is_known_spl_token_id); let mut transaction_balances: Vec = vec![]; if has_token_program { for (index, account_id) in account_keys.iter().enumerate() { - if transaction.transaction.message().is_invoked(index) - || is_known_spl_token_id(account_id) - { + if transaction.message().is_invoked(index) || is_known_spl_token_id(account_id) { continue; } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 2ebcdc9f883afb..6953d1bca04336 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3745,7 +3745,7 @@ pub mod rpc_full { let transaction = sanitize_transaction(unsanitized_tx, bank)?; let transaction = ExtendedSanitizedTransaction::from(transaction); if sig_verify { - verify_transaction(&transaction.transaction, &bank.feature_set)?; + verify_transaction(&transaction.transaction(), &bank.feature_set)?; } let TransactionSimulationResult { @@ -3757,7 +3757,7 @@ pub mod rpc_full { inner_instructions, } = bank.simulate_transaction(&transaction, enable_cpi_recording); - let account_keys = transaction.transaction.message().account_keys(); + let account_keys = transaction.message().account_keys(); let number_of_accounts = account_keys.len(); let accounts = if let Some(config_accounts) = config_accounts { diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 055daeea04e976..8fae3cbbee5688 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -110,16 +110,15 @@ impl TransactionStatusService { } Some(DurableNonceFee::Invalid) => None, None => bank.get_lamports_per_signature_for_blockhash( - transaction.transaction.message().recent_blockhash(), + transaction.message().recent_blockhash(), ), } .expect("lamports_per_signature must be available"); let fee = bank.get_fee_for_message_with_lamports_per_signature( - transaction.transaction.message(), + transaction.message(), lamports_per_signature, ); - let tx_account_locks = - transaction.transaction.get_account_locks_unchecked(); + let tx_account_locks = transaction.get_account_locks_unchecked(); let inner_instructions = inner_instructions.map(|inner_instructions| { map_inner_instructions(inner_instructions).collect() @@ -139,7 +138,7 @@ impl TransactionStatusService { }) .collect(), ); - let loaded_addresses = transaction.transaction.get_loaded_addresses(); + let loaded_addresses = transaction.get_loaded_addresses(); let mut transaction_status_meta = TransactionStatusMeta { status, fee, @@ -159,9 +158,9 @@ impl TransactionStatusService { transaction_notifier.notify_transaction( slot, transaction_index, - transaction.transaction.signature(), + transaction.signature(), &transaction_status_meta, - &transaction.transaction, + transaction.transaction(), ); } @@ -173,22 +172,16 @@ impl TransactionStatusService { } if enable_rpc_transaction_history { - if let Some(memos) = - extract_and_fmt_memos(transaction.transaction.message()) - { + if let Some(memos) = extract_and_fmt_memos(transaction.message()) { blockstore - .write_transaction_memos( - transaction.transaction.signature(), - slot, - memos, - ) + .write_transaction_memos(transaction.signature(), slot, memos) .expect("Expect database write to succeed: TransactionMemos"); } blockstore .write_transaction_status( slot, - *transaction.transaction.signature(), + *transaction.signature(), tx_account_locks.writable, tx_account_locks.readonly, transaction_status_meta, diff --git a/runtime-transaction/src/extended_transaction.rs b/runtime-transaction/src/extended_transaction.rs index 13ca194d13dc32..f3d7e67356fe1c 100644 --- a/runtime-transaction/src/extended_transaction.rs +++ b/runtime-transaction/src/extended_transaction.rs @@ -1,9 +1,19 @@ -use {solana_sdk::transaction::SanitizedTransaction, std::time::Instant}; +use { + solana_sdk::{ + hash::Hash, + message::{v0::LoadedAddresses, SanitizedMessage}, + signature::Signature, + transaction::{ + Result, SanitizedTransaction, TransactionAccountLocks, VersionedTransaction, + }, + }, + std::time::Instant, +}; /// Sanitized transaction with optional start_time #[derive(Debug, Clone, Eq, PartialEq)] pub struct ExtendedSanitizedTransaction { - pub transaction: SanitizedTransaction, + transaction: SanitizedTransaction, pub start_time: Option, } @@ -15,3 +25,72 @@ impl From for ExtendedSanitizedTransaction { } } } + +impl ExtendedSanitizedTransaction { + pub fn transaction(&self) -> &SanitizedTransaction { + &self.transaction + } + + /// Return the first signature for this transaction. + /// + /// Notes: + /// + /// Sanitized transactions must have at least one signature because the + /// number of signatures must be greater than or equal to the message header + /// value `num_required_signatures` which must be greater than 0 itself. + #[inline(always)] + pub fn signature(&self) -> &Signature { + &self.transaction.signature() + } + + /// Return the list of signatures for this transaction + #[inline(always)] + pub fn signatures(&self) -> &[Signature] { + &self.transaction.signatures() + } + + /// Return the signed message + #[inline(always)] + pub fn message(&self) -> &SanitizedMessage { + &self.transaction.message() + } + + /// Return the hash of the signed message + #[inline(always)] + pub fn message_hash(&self) -> &Hash { + &self.transaction.message_hash() + } + + /// Returns true if this transaction is a simple vote + #[inline(always)] + pub fn is_simple_vote_transaction(&self) -> bool { + self.transaction.is_simple_vote_transaction() + } + + /// Convert this sanitized transaction into a versioned transaction for + /// recording in the ledger. + #[inline(always)] + pub fn to_versioned_transaction(&self) -> VersionedTransaction { + self.transaction.to_versioned_transaction() + } + + /// Validate and return the account keys locked by this transaction + #[inline(always)] + pub fn get_account_locks( + &self, + tx_account_lock_limit: usize, + ) -> Result { + self.transaction.get_account_locks(tx_account_lock_limit) + } + + /// Return the list of accounts that must be locked during processing this transaction. + #[inline(always)] + pub fn get_account_locks_unchecked(&self) -> TransactionAccountLocks { + self.transaction.get_account_locks_unchecked() + } + + /// Return the list of addresses loaded from on-chain address lookup tables + pub fn get_loaded_addresses(&self) -> LoadedAddresses { + self.transaction.get_loaded_addresses() + } +} diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d1e22ff7162647..e05ff46f29de9c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4092,8 +4092,8 @@ impl Bank { // Add the message hash to the status cache to ensure that this message // won't be processed again with a different signature. status_cache.insert( - tx.transaction.message().recent_blockhash(), - tx.transaction.message_hash(), + tx.message().recent_blockhash(), + tx.message_hash(), self.slot(), details.status.clone(), ); @@ -4101,8 +4101,8 @@ impl Bank { // can be queried by transaction signature over RPC. In the future, this should // only be added for API nodes because voting validators don't need to do this. status_cache.insert( - tx.transaction.message().recent_blockhash(), - tx.transaction.signature(), + tx.message().recent_blockhash(), + tx.signature(), self.slot(), details.status.clone(), ); @@ -4204,12 +4204,8 @@ impl Bank { let sanitized_txs = txs .into_iter() .map(|tx| { - SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self).map(|txn| { - ExtendedSanitizedTransaction { - transaction: txn, - start_time: None, - } - }) + SanitizedTransaction::try_create(tx, MessageHash::Compute, None, self) + .map(|txn| txn.into()) }) .collect::>>()?; let tx_account_lock_limit = self.get_transaction_account_lock_limit(); @@ -4261,7 +4257,6 @@ impl Bank { ) -> TransactionBatch<'_, '_> { let tx_account_lock_limit = self.get_transaction_account_lock_limit(); let lock_result = transaction - .transaction .get_account_locks(tx_account_lock_limit) .map(|_| ()); let mut batch = TransactionBatch::new( @@ -4291,7 +4286,7 @@ impl Bank { transaction: &ExtendedSanitizedTransaction, enable_cpi_recording: bool, ) -> TransactionSimulationResult { - let account_keys = transaction.transaction.message().account_keys(); + let account_keys = transaction.message().account_keys(); let number_of_accounts = account_keys.len(); let account_overrides = self.get_account_overrides_for_simulation(&account_keys); let batch = self.prepare_unlocked_batch_from_single_tx(transaction); @@ -4419,7 +4414,7 @@ impl Bank { .zip(lock_results) .map(|(tx, lock_res)| match lock_res { Ok(()) => self.check_transaction_age( - &tx.borrow().transaction, + tx.borrow().transaction(), max_age, &next_durable_nonce, &hash_queue, @@ -4482,7 +4477,7 @@ impl Bank { .map(|(sanitized_tx, (lock_result, nonce, lamports))| { let sanitized_tx = sanitized_tx.borrow(); if lock_result.is_ok() - && self.is_transaction_already_processed(&sanitized_tx.transaction, &rcache) + && self.is_transaction_already_processed(&sanitized_tx.transaction(), &rcache) { error_counters.already_processed += 1; return (Err(TransactionError::AlreadyProcessed), None, None); @@ -4548,7 +4543,7 @@ impl Bank { let mut balances: TransactionBalances = vec![]; for transaction in batch.sanitized_transactions() { let mut transaction_balances: Vec = vec![]; - for account_key in transaction.transaction.message().account_keys().iter() { + for account_key in transaction.message().account_keys().iter() { transaction_balances.push(self.get_balance(account_key)); } balances.push(transaction_balances); @@ -4647,7 +4642,7 @@ impl Bank { let mut collect_logs_time = Measure::start("collect_logs_time"); for (execution_result, tx) in sanitized_output.execution_results.iter().zip(sanitized_txs) { if let Some(debug_keys) = &self.transaction_debug_keys { - for key in tx.transaction.message().account_keys().iter() { + for key in tx.message().account_keys().iter() { if debug_keys.contains(key) { let result = execution_result.flattened_result(); info!("slot: {} result: {:?} tx: {:?}", self.slot, result, tx); @@ -4656,7 +4651,7 @@ impl Bank { } } - let is_vote = tx.transaction.is_simple_vote_transaction(); + let is_vote = tx.is_simple_vote_transaction(); if execution_result.was_executed() // Skip log collection for unprocessed transactions && transaction_log_collector_config.filter != TransactionLogCollectorFilter::None @@ -4666,7 +4661,7 @@ impl Bank { .mentioned_addresses .is_empty() { - for key in tx.transaction.message().account_keys().iter() { + for key in tx.message().account_keys().iter() { if transaction_log_collector_config .mentioned_addresses .contains(key) @@ -4699,7 +4694,7 @@ impl Bank { let transaction_log_index = transaction_log_collector.logs.len(); transaction_log_collector.logs.push(TransactionLogInfo { - signature: *tx.transaction.signature(), + signature: *tx.signature(), result: status.clone(), is_vote, log_messages: log_messages.clone(), @@ -4719,8 +4714,7 @@ impl Bank { // Signature count must be accumulated only if the transaction // is executed, otherwise a mismatched count between banking and // replay could occur - signature_count += - u64::from(tx.transaction.message().header().num_required_signatures); + signature_count += u64::from(tx.message().header().num_required_signatures); executed_transactions_count += 1; } @@ -4852,9 +4846,7 @@ impl Bank { .map(|maybe_lamports_per_signature| (maybe_lamports_per_signature, true)) .unwrap_or_else(|| { ( - hash_queue.get_lamports_per_signature( - tx.transaction.message().recent_blockhash(), - ), + hash_queue.get_lamports_per_signature(tx.message().recent_blockhash()), false, ) }); @@ -4862,7 +4854,7 @@ impl Bank { let lamports_per_signature = lamports_per_signature.ok_or(TransactionError::BlockhashNotFound)?; let fee = self.get_fee_for_message_with_lamports_per_signature( - tx.transaction.message(), + tx.message(), lamports_per_signature, ); @@ -4874,7 +4866,7 @@ impl Bank { // post-load, fee deducted, pre-execute account state // stored if execution_status.is_err() && !is_nonce { - self.withdraw(tx.transaction.message().fee_payer(), fee)?; + self.withdraw(tx.message().fee_payer(), fee)?; } fees += fee; @@ -6944,7 +6936,7 @@ impl Bank { .filter(|(_, execution_result, _)| execution_result.was_executed_successfully()) .flat_map(|(tx, _, (load_result, _))| { load_result.iter().flat_map(|loaded_transaction| { - let num_account_keys = tx.transaction.message().account_keys().len(); + let num_account_keys = tx.message().account_keys().len(); loaded_transaction.accounts.iter().take(num_account_keys) }) }) @@ -7689,10 +7681,7 @@ impl Bank { let transaction_account_lock_limit = self.get_transaction_account_lock_limit(); let sanitized_txs = txs .into_iter() - .map(|txn| ExtendedSanitizedTransaction { - transaction: SanitizedTransaction::from_transaction_for_tests(txn), - start_time: None, - }) + .map(|txn| SanitizedTransaction::from_transaction_for_tests(txn).into()) .collect::>(); let lock_results = self .rc diff --git a/runtime/src/bank_utils.rs b/runtime/src/bank_utils.rs index 618089e10b55b7..80b4ee21b0f9fd 100644 --- a/runtime/src/bank_utils.rs +++ b/runtime/src/bank_utils.rs @@ -49,10 +49,9 @@ pub fn find_and_send_votes( .iter() .zip(execution_results.iter()) .for_each(|(tx, result)| { - if tx.transaction.is_simple_vote_transaction() && result.was_executed_successfully() - { + if tx.is_simple_vote_transaction() && result.was_executed_successfully() { if let Some(parsed_vote) = - vote_parser::parse_sanitized_vote_transaction(&tx.transaction) + vote_parser::parse_sanitized_vote_transaction(tx.transaction()) { if parsed_vote.1.last_voted_slot().is_some() { let _ = vote_sender.send(parsed_vote); diff --git a/runtime/src/compute_budget_details.rs b/runtime/src/compute_budget_details.rs index 72b10a11b33bcc..fb403720f0c8f7 100644 --- a/runtime/src/compute_budget_details.rs +++ b/runtime/src/compute_budget_details.rs @@ -1,5 +1,6 @@ use { solana_program_runtime::compute_budget_processor::process_compute_budget_instructions, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ instruction::CompiledInstruction, pubkey::Pubkey, @@ -55,6 +56,16 @@ impl GetComputeBudgetDetails for SanitizedTransaction { } } +impl GetComputeBudgetDetails for ExtendedSanitizedTransaction { + fn get_compute_budget_details( + &self, + round_compute_unit_price_enabled: bool, + ) -> Option { + self.transaction() + .get_compute_budget_details(round_compute_unit_price_enabled) + } +} + #[cfg(test)] mod tests { use { diff --git a/runtime/src/prioritization_fee_cache.rs b/runtime/src/prioritization_fee_cache.rs index 37800293db68e3..f2022399a55da5 100644 --- a/runtime/src/prioritization_fee_cache.rs +++ b/runtime/src/prioritization_fee_cache.rs @@ -218,19 +218,14 @@ impl PrioritizationFeeCache { for sanitized_transaction in txs { // Vote transactions are not prioritized, therefore they are excluded from // updating fee_cache. - if sanitized_transaction - .transaction - .is_simple_vote_transaction() - { + if sanitized_transaction.is_simple_vote_transaction() { continue; } let round_compute_unit_price_enabled = false; // TODO: bank.feture_set.is_active(round_compute_unit_price) let compute_budget_details = sanitized_transaction - .transaction .get_compute_budget_details(round_compute_unit_price_enabled); let account_locks = sanitized_transaction - .transaction .get_account_locks(bank.get_transaction_account_lock_limit()); if compute_budget_details.is_none() || account_locks.is_err() { diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 0e266d30f10d13..ab7d27f25c0a36 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -126,7 +126,7 @@ pub(crate) fn load_accounts( .zip(lock_results) .map(|etx| match etx { (tx, (Ok(()), nonce, lamports_per_signature)) => { - let message = tx.transaction.message(); + let message = tx.message(); let fee = if let Some(lamports_per_signature) = lamports_per_signature { fee_structure.calculate_fee( message, diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 294241159628e6..515e01635b36e9 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -266,7 +266,7 @@ impl TransactionBatchProcessor { let mut compute_budget_process_transaction_time = Measure::start("compute_budget_process_transaction_time"); let maybe_compute_budget = ComputeBudget::try_from_instructions( - tx.transaction.message().program_instructions_iter(), + tx.message().program_instructions_iter(), ); compute_budget_process_transaction_time.stop(); saturating_add_assign!( @@ -283,7 +283,7 @@ impl TransactionBatchProcessor { let result = self.execute_loaded_transaction( callbacks, - &tx.transaction, + tx.transaction(), loaded_transaction, compute_budget, nonce.as_ref().map(DurableNonceFee::from), @@ -362,8 +362,7 @@ impl TransactionBatchProcessor { lock_results.iter_mut().zip(txs).for_each(|etx| { if let ((Ok(()), _nonce, lamports_per_signature), tx) = etx { if lamports_per_signature.is_some() { - tx.transaction - .message() + tx.message() .account_keys() .iter() .for_each(|key| match result.entry(*key) { From ad6896d2aa45c4f71b7bac86b7a30e99fb105315 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 18:06:46 -0700 Subject: [PATCH 36/42] clippy fixes --- runtime-transaction/src/extended_transaction.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/runtime-transaction/src/extended_transaction.rs b/runtime-transaction/src/extended_transaction.rs index f3d7e67356fe1c..5ac63af8d4f36d 100644 --- a/runtime-transaction/src/extended_transaction.rs +++ b/runtime-transaction/src/extended_transaction.rs @@ -40,25 +40,25 @@ impl ExtendedSanitizedTransaction { /// value `num_required_signatures` which must be greater than 0 itself. #[inline(always)] pub fn signature(&self) -> &Signature { - &self.transaction.signature() + self.transaction.signature() } /// Return the list of signatures for this transaction #[inline(always)] pub fn signatures(&self) -> &[Signature] { - &self.transaction.signatures() + self.transaction.signatures() } /// Return the signed message #[inline(always)] pub fn message(&self) -> &SanitizedMessage { - &self.transaction.message() + self.transaction.message() } /// Return the hash of the signed message #[inline(always)] pub fn message_hash(&self) -> &Hash { - &self.transaction.message_hash() + self.transaction.message_hash() } /// Returns true if this transaction is a simple vote @@ -90,6 +90,7 @@ impl ExtendedSanitizedTransaction { } /// Return the list of addresses loaded from on-chain address lookup tables + #[inline(always)] pub fn get_loaded_addresses(&self) -> LoadedAddresses { self.transaction.get_loaded_addresses() } From fe646571bebcb883bad98344d2138f10d82ff1d4 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Tue, 12 Mar 2024 18:28:06 -0700 Subject: [PATCH 37/42] clippy fixes --- rpc/src/rpc.rs | 2 +- runtime/src/bank.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 6953d1bca04336..6b921ae990fdfb 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3745,7 +3745,7 @@ pub mod rpc_full { let transaction = sanitize_transaction(unsanitized_tx, bank)?; let transaction = ExtendedSanitizedTransaction::from(transaction); if sig_verify { - verify_transaction(&transaction.transaction(), &bank.feature_set)?; + verify_transaction(transaction.transaction(), &bank.feature_set)?; } let TransactionSimulationResult { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e05ff46f29de9c..14a8ec6137a12a 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4477,7 +4477,7 @@ impl Bank { .map(|(sanitized_tx, (lock_result, nonce, lamports))| { let sanitized_tx = sanitized_tx.borrow(); if lock_result.is_ok() - && self.is_transaction_already_processed(&sanitized_tx.transaction(), &rcache) + && self.is_transaction_already_processed(sanitized_tx.transaction(), &rcache) { error_counters.already_processed += 1; return (Err(TransactionError::AlreadyProcessed), None, None); From 26437de5642260771f20d7506f19e1e3e1f4da72 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Wed, 13 Mar 2024 01:00:54 -0700 Subject: [PATCH 38/42] clippy fixes; dependency sorting --- ledger/Cargo.toml | 2 +- svm/Cargo.toml | 2 +- unified-scheduler-logic/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ledger/Cargo.toml b/ledger/Cargo.toml index 8d3de706183b4e..9ebe4743bc6a7b 100644 --- a/ledger/Cargo.toml +++ b/ledger/Cargo.toml @@ -49,8 +49,8 @@ solana-metrics = { workspace = true } solana-perf = { workspace = true } solana-program-runtime = { workspace = true } solana-rayon-threadlimit = { workspace = true } -solana-runtime-transaction = { workspace = true } solana-runtime = { workspace = true } +solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-stake-program = { workspace = true } solana-storage-bigtable = { workspace = true } diff --git a/svm/Cargo.toml b/svm/Cargo.toml index b436236ac5ba0d..4d5bb9f17781b0 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -10,8 +10,8 @@ license = { workspace = true } edition = { workspace = true } [dependencies] -itertools = { workspace = true } histogram = { workspace = true } +itertools = { workspace = true } log = { workspace = true } percentage = { workspace = true } solana-bpf-loader-program = { workspace = true } diff --git a/unified-scheduler-logic/Cargo.toml b/unified-scheduler-logic/Cargo.toml index 0cc315b159af39..60d4a74dd1db74 100644 --- a/unified-scheduler-logic/Cargo.toml +++ b/unified-scheduler-logic/Cargo.toml @@ -10,5 +10,5 @@ license = { workspace = true } edition = { workspace = true } [dependencies] -solana-sdk = { workspace = true } solana-runtime-transaction = { workspace = true } +solana-sdk = { workspace = true } From 5f0c223d889432543c845c8244d1b706a4c694eb Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Wed, 13 Mar 2024 01:37:13 -0700 Subject: [PATCH 39/42] initialize start_time in ExtendedSanitizedTransaction --- .../unprocessed_transaction_storage.rs | 5 ++++- runtime-transaction/src/extended_transaction.rs | 13 ++++++++++++- svm/src/transaction_processor.rs | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 02d61c42b894b7..7aa9dad8ad3cc1 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -211,7 +211,10 @@ fn consume_scan_should_process_packet( payload .sanitized_transactions - .push(ExtendedSanitizedTransaction::from(sanitized_transaction)); + .push(ExtendedSanitizedTransaction::new( + sanitized_transaction, + packet.start_time().clone(), + )); ProcessingDecision::Now } else { payload diff --git a/runtime-transaction/src/extended_transaction.rs b/runtime-transaction/src/extended_transaction.rs index 5ac63af8d4f36d..ea85920d984630 100644 --- a/runtime-transaction/src/extended_transaction.rs +++ b/runtime-transaction/src/extended_transaction.rs @@ -14,7 +14,7 @@ use { #[derive(Debug, Clone, Eq, PartialEq)] pub struct ExtendedSanitizedTransaction { transaction: SanitizedTransaction, - pub start_time: Option, + start_time: Option, } impl From for ExtendedSanitizedTransaction { @@ -27,10 +27,21 @@ impl From for ExtendedSanitizedTransaction { } impl ExtendedSanitizedTransaction { + pub fn new(transaction: SanitizedTransaction, start_time: Option) -> Self { + Self { + transaction, + start_time, + } + } + pub fn transaction(&self) -> &SanitizedTransaction { &self.transaction } + pub fn start_time(&self) -> &Option { + &self.start_time + } + /// Return the first signature for this transaction. /// /// Notes: diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 515e01635b36e9..9f457361e64041 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -300,7 +300,7 @@ impl TransactionBatchProcessor { } = &result { if let Some(perf_track_metrics) = perf_track_metrics.as_mut() { - if let Some(start_time) = &tx.start_time { + if let Some(start_time) = tx.start_time() { // measure the time from start of banking stage to the execution of the transaction let duration = Instant::now().duration_since(*start_time); perf_track_metrics From 57fc92bbc23cfdc7386517e02855ffc753d34bbb Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Wed, 13 Mar 2024 02:37:13 -0700 Subject: [PATCH 40/42] initialize start_time in ExtendedSanitizedTransaction --- core/src/banking_stage/unprocessed_transaction_storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 7aa9dad8ad3cc1..3a4765b9041614 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -213,7 +213,7 @@ fn consume_scan_should_process_packet( .sanitized_transactions .push(ExtendedSanitizedTransaction::new( sanitized_transaction, - packet.start_time().clone(), + *packet.start_time(), )); ProcessingDecision::Now } else { From 5bdb868124bfe5005ec76a93688f6f822d916856 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Wed, 13 Mar 2024 17:16:35 -0700 Subject: [PATCH 41/42] Build ExtendedSanitizedTransaction in ImmutablePacket --- core/src/banking_stage/immutable_deserialized_packet.rs | 5 +++-- .../transaction_scheduler/scheduler_controller.rs | 7 +------ core/src/banking_stage/unprocessed_transaction_storage.rs | 7 +------ 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/core/src/banking_stage/immutable_deserialized_packet.rs b/core/src/banking_stage/immutable_deserialized_packet.rs index 6eb5d68ecaaca5..66fc39d11fe711 100644 --- a/core/src/banking_stage/immutable_deserialized_packet.rs +++ b/core/src/banking_stage/immutable_deserialized_packet.rs @@ -1,6 +1,7 @@ use { solana_perf::packet::Packet, solana_runtime::compute_budget_details::{ComputeBudgetDetails, GetComputeBudgetDetails}, + solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ feature_set, hash::Hash, @@ -116,7 +117,7 @@ impl ImmutableDeserializedPacket { feature_set: &Arc, votes_only: bool, address_loader: impl AddressLoader, - ) -> Option { + ) -> Option { if votes_only && !self.is_simple_vote() { return None; } @@ -128,7 +129,7 @@ impl ImmutableDeserializedPacket { ) .ok()?; tx.verify_precompiles(feature_set).ok()?; - Some(tx) + Some(ExtendedSanitizedTransaction::new(tx, *self.start_time())) } } diff --git a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs index 5206e2a5a674a5..39435d45de7c12 100644 --- a/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs +++ b/core/src/banking_stage/transaction_scheduler/scheduler_controller.rs @@ -392,12 +392,7 @@ impl SchedulerController { }) .filter_map(|tx| { process_compute_budget_instructions(tx.message().program_instructions_iter()) - .map(|compute_budget| { - ( - ExtendedSanitizedTransaction::from(tx), - compute_budget.into(), - ) - }) + .map(|compute_budget| (tx, compute_budget.into())) .ok() }) .unzip(); diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 3a4765b9041614..366c8a542afe01 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -209,12 +209,7 @@ fn consume_scan_should_process_packet( return ProcessingDecision::Later; } - payload - .sanitized_transactions - .push(ExtendedSanitizedTransaction::new( - sanitized_transaction, - *packet.start_time(), - )); + payload.sanitized_transactions.push(sanitized_transaction); ProcessingDecision::Now } else { payload From 53dee5b9024873e4f9e48588a7a326b52400ab54 Mon Sep 17 00:00:00 2001 From: Lijun Wang <83639177+lijunwangs@users.noreply.github.com> Date: Wed, 13 Mar 2024 17:31:56 -0700 Subject: [PATCH 42/42] Fixed tracked packet performance metrics for central scheduler --- core/src/banking_stage/latest_unprocessed_votes.rs | 3 +-- core/src/banking_stage/unprocessed_transaction_storage.rs | 7 +------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/core/src/banking_stage/latest_unprocessed_votes.rs b/core/src/banking_stage/latest_unprocessed_votes.rs index f550c680112c98..a62e5bf9b3e455 100644 --- a/core/src/banking_stage/latest_unprocessed_votes.rs +++ b/core/src/banking_stage/latest_unprocessed_votes.rs @@ -7,7 +7,6 @@ use { rand::{thread_rng, Rng}, solana_perf::packet::Packet, solana_runtime::bank::Bank, - solana_runtime_transaction::extended_transaction::ExtendedSanitizedTransaction, solana_sdk::{ clock::{Slot, UnixTimestamp}, program_utils::limited_deserialize, @@ -287,7 +286,7 @@ impl LatestUnprocessedVotes { ) { if forward_packet_batches_by_accounts.try_add_packet( - &ExtendedSanitizedTransaction::from(sanitized_vote_transaction), + &sanitized_vote_transaction, deserialized_vote_packet, &bank.feature_set, ) { diff --git a/core/src/banking_stage/unprocessed_transaction_storage.rs b/core/src/banking_stage/unprocessed_transaction_storage.rs index 366c8a542afe01..e606c52a63beca 100644 --- a/core/src/banking_stage/unprocessed_transaction_storage.rs +++ b/core/src/banking_stage/unprocessed_transaction_storage.rs @@ -774,12 +774,7 @@ impl ThreadLocalUnprocessedPackets { .filter_map(|(packet_index, deserialized_packet)| { deserialized_packet .build_sanitized_transaction(&bank.feature_set, bank.vote_only_bank(), bank) - .map(|transaction| { - ( - ExtendedSanitizedTransaction::from(transaction), - packet_index, - ) - }) + .map(|transaction| (transaction, packet_index)) }) .unzip();