* Discard packets statically known to fail (#370)
* Discard packets statically known to fail
* add test
(cherry picked from commit 5f1693224e
)
# Conflicts:
# core/src/banking_stage/transaction_scheduler/scheduler_controller.rs
* resolve conflict
---------
Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
This commit is contained in:
parent
adc63fb3c0
commit
749c529d9e
|
@ -1,4 +1,5 @@
|
|||
use {
|
||||
solana_cost_model::block_cost_limits::BUILT_IN_INSTRUCTION_COSTS,
|
||||
solana_perf::packet::Packet,
|
||||
solana_runtime::transaction_priority_details::{
|
||||
GetTransactionPriorityDetails, TransactionPriorityDetails,
|
||||
|
@ -8,6 +9,7 @@ use {
|
|||
hash::Hash,
|
||||
message::Message,
|
||||
sanitize::SanitizeError,
|
||||
saturating_add_assign,
|
||||
short_vec::decode_shortu16_len,
|
||||
signature::Signature,
|
||||
transaction::{
|
||||
|
@ -96,6 +98,22 @@ impl ImmutableDeserializedPacket {
|
|||
self.priority_details.compute_unit_limit
|
||||
}
|
||||
|
||||
/// Returns true if the transaction's compute unit limit is at least as
|
||||
/// large as the sum of the static builtins' costs.
|
||||
/// This is a simple sanity check so the leader can discard transactions
|
||||
/// which are statically known to exceed the compute budget, and will
|
||||
/// result in no useful state-change.
|
||||
pub fn compute_unit_limit_above_static_builtins(&self) -> bool {
|
||||
let mut static_builtin_cost_sum: u64 = 0;
|
||||
for (program_id, _) in self.transaction.get_message().program_instructions_iter() {
|
||||
if let Some(ix_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) {
|
||||
saturating_add_assign!(static_builtin_cost_sum, *ix_cost);
|
||||
}
|
||||
}
|
||||
|
||||
self.compute_unit_limit() >= static_builtin_cost_sum
|
||||
}
|
||||
|
||||
// This function deserializes packets into transactions, computes the blake3 hash of transaction
|
||||
// messages, and verifies secp256k1 instructions.
|
||||
pub fn build_sanitized_transaction(
|
||||
|
@ -148,7 +166,10 @@ fn packet_message(packet: &Packet) -> Result<&[u8], DeserializedPacketError> {
|
|||
mod tests {
|
||||
use {
|
||||
super::*,
|
||||
solana_sdk::{signature::Keypair, system_transaction},
|
||||
solana_sdk::{
|
||||
compute_budget, instruction::Instruction, pubkey::Pubkey, signature::Keypair,
|
||||
signer::Signer, system_instruction, system_transaction, transaction::Transaction,
|
||||
},
|
||||
};
|
||||
|
||||
#[test]
|
||||
|
@ -164,4 +185,33 @@ mod tests {
|
|||
|
||||
assert!(deserialized_packet.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compute_unit_limit_above_static_builtins() {
|
||||
// Cases:
|
||||
// 1. compute_unit_limit under static builtins
|
||||
// 2. compute_unit_limit equal to static builtins
|
||||
// 3. compute_unit_limit above static builtins
|
||||
for (cu_limit, expectation) in [(250, false), (300, true), (350, true)] {
|
||||
let keypair = Keypair::new();
|
||||
let bpf_program_id = Pubkey::new_unique();
|
||||
let ixs = vec![
|
||||
system_instruction::transfer(&keypair.pubkey(), &Pubkey::new_unique(), 1),
|
||||
compute_budget::ComputeBudgetInstruction::set_compute_unit_limit(cu_limit),
|
||||
Instruction::new_with_bytes(bpf_program_id, &[], vec![]), // non-builtin - not counted in filter
|
||||
];
|
||||
let tx = Transaction::new_signed_with_payer(
|
||||
&ixs,
|
||||
Some(&keypair.pubkey()),
|
||||
&[&keypair],
|
||||
Hash::new_unique(),
|
||||
);
|
||||
let packet = Packet::from_data(None, tx).unwrap();
|
||||
let deserialized_packet = ImmutableDeserializedPacket::new(packet).unwrap();
|
||||
assert_eq!(
|
||||
deserialized_packet.compute_unit_limit_above_static_builtins(),
|
||||
expectation
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -50,6 +50,7 @@ impl PacketDeserializer {
|
|||
&self,
|
||||
recv_timeout: Duration,
|
||||
capacity: usize,
|
||||
packet_filter: impl Fn(&ImmutableDeserializedPacket) -> bool,
|
||||
) -> Result<ReceivePacketResults, RecvTimeoutError> {
|
||||
let (packet_count, packet_batches) = self.receive_until(recv_timeout, capacity)?;
|
||||
|
||||
|
@ -62,6 +63,7 @@ impl PacketDeserializer {
|
|||
packet_count,
|
||||
&packet_batches,
|
||||
round_compute_unit_price_enabled,
|
||||
&packet_filter,
|
||||
))
|
||||
}
|
||||
|
||||
|
@ -71,6 +73,7 @@ impl PacketDeserializer {
|
|||
packet_count: usize,
|
||||
banking_batches: &[BankingPacketBatch],
|
||||
round_compute_unit_price_enabled: bool,
|
||||
packet_filter: &impl Fn(&ImmutableDeserializedPacket) -> bool,
|
||||
) -> ReceivePacketResults {
|
||||
let mut passed_sigverify_count: usize = 0;
|
||||
let mut failed_sigverify_count: usize = 0;
|
||||
|
@ -88,6 +91,7 @@ impl PacketDeserializer {
|
|||
packet_batch,
|
||||
&packet_indexes,
|
||||
round_compute_unit_price_enabled,
|
||||
packet_filter,
|
||||
));
|
||||
}
|
||||
|
||||
|
@ -158,13 +162,16 @@ impl PacketDeserializer {
|
|||
packet_batch: &'a PacketBatch,
|
||||
packet_indexes: &'a [usize],
|
||||
round_compute_unit_price_enabled: bool,
|
||||
packet_filter: &'a (impl Fn(&ImmutableDeserializedPacket) -> bool + 'a),
|
||||
) -> impl Iterator<Item = ImmutableDeserializedPacket> + 'a {
|
||||
packet_indexes.iter().filter_map(move |packet_index| {
|
||||
let mut packet_clone = packet_batch[*packet_index].clone();
|
||||
packet_clone
|
||||
.meta_mut()
|
||||
.set_round_compute_unit_price(round_compute_unit_price_enabled);
|
||||
ImmutableDeserializedPacket::new(packet_clone).ok()
|
||||
ImmutableDeserializedPacket::new(packet_clone)
|
||||
.ok()
|
||||
.filter(packet_filter)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
@ -186,7 +193,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn test_deserialize_and_collect_packets_empty() {
|
||||
let results = PacketDeserializer::deserialize_and_collect_packets(0, &[], false);
|
||||
let results = PacketDeserializer::deserialize_and_collect_packets(0, &[], false, &|_| true);
|
||||
assert_eq!(results.deserialized_packets.len(), 0);
|
||||
assert!(results.new_tracer_stats_option.is_none());
|
||||
assert_eq!(results.passed_sigverify_count, 0);
|
||||
|
@ -204,6 +211,7 @@ mod tests {
|
|||
packet_count,
|
||||
&[BankingPacketBatch::new((packet_batches, None))],
|
||||
false,
|
||||
&|_| true,
|
||||
);
|
||||
assert_eq!(results.deserialized_packets.len(), 2);
|
||||
assert!(results.new_tracer_stats_option.is_none());
|
||||
|
@ -223,6 +231,7 @@ mod tests {
|
|||
packet_count,
|
||||
&[BankingPacketBatch::new((packet_batches, None))],
|
||||
false,
|
||||
&|_| true,
|
||||
);
|
||||
assert_eq!(results.deserialized_packets.len(), 1);
|
||||
assert!(results.new_tracer_stats_option.is_none());
|
||||
|
|
|
@ -49,6 +49,7 @@ impl PacketReceiver {
|
|||
.receive_packets(
|
||||
recv_timeout,
|
||||
unprocessed_transaction_storage.max_receive_size(),
|
||||
|packet| packet.compute_unit_limit_above_static_builtins(),
|
||||
)
|
||||
// Consumes results if Ok, otherwise we keep the Err
|
||||
.map(|receive_packet_results| {
|
||||
|
|
Loading…
Reference in New Issue