Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit a67767a

Browse files
mergify[bot]apfitzge
authored andcommitted
v1.17: Discard packets statically known to fail (backport of #370) (#374)
* Discard packets statically known to fail (#370) * Discard packets statically known to fail * add test (cherry picked from commit 5f16932) # Conflicts: # core/src/banking_stage/transaction_scheduler/scheduler_controller.rs * resolve conflict --------- Co-authored-by: Andrew Fitzgerald <[email protected]>
1 parent 845f0d4 commit a67767a

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

core/src/banking_stage/immutable_deserialized_packet.rs

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use {
2+
solana_cost_model::block_cost_limits::BUILT_IN_INSTRUCTION_COSTS,
23
solana_perf::packet::Packet,
34
solana_runtime::transaction_priority_details::{
45
GetTransactionPriorityDetails, TransactionPriorityDetails,
@@ -8,6 +9,7 @@ use {
89
hash::Hash,
910
message::Message,
1011
sanitize::SanitizeError,
12+
saturating_add_assign,
1113
short_vec::decode_shortu16_len,
1214
signature::Signature,
1315
transaction::{
@@ -96,6 +98,22 @@ impl ImmutableDeserializedPacket {
9698
self.priority_details.compute_unit_limit
9799
}
98100

101+
/// Returns true if the transaction's compute unit limit is at least as
102+
/// large as the sum of the static builtins' costs.
103+
/// This is a simple sanity check so the leader can discard transactions
104+
/// which are statically known to exceed the compute budget, and will
105+
/// result in no useful state-change.
106+
pub fn compute_unit_limit_above_static_builtins(&self) -> bool {
107+
let mut static_builtin_cost_sum: u64 = 0;
108+
for (program_id, _) in self.transaction.get_message().program_instructions_iter() {
109+
if let Some(ix_cost) = BUILT_IN_INSTRUCTION_COSTS.get(program_id) {
110+
saturating_add_assign!(static_builtin_cost_sum, *ix_cost);
111+
}
112+
}
113+
114+
self.compute_unit_limit() >= static_builtin_cost_sum
115+
}
116+
99117
// This function deserializes packets into transactions, computes the blake3 hash of transaction
100118
// messages, and verifies secp256k1 instructions.
101119
pub fn build_sanitized_transaction(
@@ -148,7 +166,10 @@ fn packet_message(packet: &Packet) -> Result<&[u8], DeserializedPacketError> {
148166
mod tests {
149167
use {
150168
super::*,
151-
solana_sdk::{signature::Keypair, system_transaction},
169+
solana_sdk::{
170+
compute_budget, instruction::Instruction, pubkey::Pubkey, signature::Keypair,
171+
signer::Signer, system_instruction, system_transaction, transaction::Transaction,
172+
},
152173
};
153174

154175
#[test]
@@ -164,4 +185,33 @@ mod tests {
164185

165186
assert!(deserialized_packet.is_ok());
166187
}
188+
189+
#[test]
190+
fn compute_unit_limit_above_static_builtins() {
191+
// Cases:
192+
// 1. compute_unit_limit under static builtins
193+
// 2. compute_unit_limit equal to static builtins
194+
// 3. compute_unit_limit above static builtins
195+
for (cu_limit, expectation) in [(250, false), (300, true), (350, true)] {
196+
let keypair = Keypair::new();
197+
let bpf_program_id = Pubkey::new_unique();
198+
let ixs = vec![
199+
system_instruction::transfer(&keypair.pubkey(), &Pubkey::new_unique(), 1),
200+
compute_budget::ComputeBudgetInstruction::set_compute_unit_limit(cu_limit),
201+
Instruction::new_with_bytes(bpf_program_id, &[], vec![]), // non-builtin - not counted in filter
202+
];
203+
let tx = Transaction::new_signed_with_payer(
204+
&ixs,
205+
Some(&keypair.pubkey()),
206+
&[&keypair],
207+
Hash::new_unique(),
208+
);
209+
let packet = Packet::from_data(None, tx).unwrap();
210+
let deserialized_packet = ImmutableDeserializedPacket::new(packet).unwrap();
211+
assert_eq!(
212+
deserialized_packet.compute_unit_limit_above_static_builtins(),
213+
expectation
214+
);
215+
}
216+
}
167217
}

core/src/banking_stage/packet_deserializer.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ impl PacketDeserializer {
5050
&self,
5151
recv_timeout: Duration,
5252
capacity: usize,
53+
packet_filter: impl Fn(&ImmutableDeserializedPacket) -> bool,
5354
) -> Result<ReceivePacketResults, RecvTimeoutError> {
5455
let (packet_count, packet_batches) = self.receive_until(recv_timeout, capacity)?;
5556

@@ -62,6 +63,7 @@ impl PacketDeserializer {
6263
packet_count,
6364
&packet_batches,
6465
round_compute_unit_price_enabled,
66+
&packet_filter,
6567
))
6668
}
6769

@@ -71,6 +73,7 @@ impl PacketDeserializer {
7173
packet_count: usize,
7274
banking_batches: &[BankingPacketBatch],
7375
round_compute_unit_price_enabled: bool,
76+
packet_filter: &impl Fn(&ImmutableDeserializedPacket) -> bool,
7477
) -> ReceivePacketResults {
7578
let mut passed_sigverify_count: usize = 0;
7679
let mut failed_sigverify_count: usize = 0;
@@ -88,6 +91,7 @@ impl PacketDeserializer {
8891
packet_batch,
8992
&packet_indexes,
9093
round_compute_unit_price_enabled,
94+
packet_filter,
9195
));
9296
}
9397

@@ -158,13 +162,16 @@ impl PacketDeserializer {
158162
packet_batch: &'a PacketBatch,
159163
packet_indexes: &'a [usize],
160164
round_compute_unit_price_enabled: bool,
165+
packet_filter: &'a (impl Fn(&ImmutableDeserializedPacket) -> bool + 'a),
161166
) -> impl Iterator<Item = ImmutableDeserializedPacket> + 'a {
162167
packet_indexes.iter().filter_map(move |packet_index| {
163168
let mut packet_clone = packet_batch[*packet_index].clone();
164169
packet_clone
165170
.meta_mut()
166171
.set_round_compute_unit_price(round_compute_unit_price_enabled);
167-
ImmutableDeserializedPacket::new(packet_clone).ok()
172+
ImmutableDeserializedPacket::new(packet_clone)
173+
.ok()
174+
.filter(packet_filter)
168175
})
169176
}
170177
}
@@ -186,7 +193,7 @@ mod tests {
186193

187194
#[test]
188195
fn test_deserialize_and_collect_packets_empty() {
189-
let results = PacketDeserializer::deserialize_and_collect_packets(0, &[], false);
196+
let results = PacketDeserializer::deserialize_and_collect_packets(0, &[], false, &|_| true);
190197
assert_eq!(results.deserialized_packets.len(), 0);
191198
assert!(results.new_tracer_stats_option.is_none());
192199
assert_eq!(results.passed_sigverify_count, 0);
@@ -204,6 +211,7 @@ mod tests {
204211
packet_count,
205212
&[BankingPacketBatch::new((packet_batches, None))],
206213
false,
214+
&|_| true,
207215
);
208216
assert_eq!(results.deserialized_packets.len(), 2);
209217
assert!(results.new_tracer_stats_option.is_none());
@@ -223,6 +231,7 @@ mod tests {
223231
packet_count,
224232
&[BankingPacketBatch::new((packet_batches, None))],
225233
false,
234+
&|_| true,
226235
);
227236
assert_eq!(results.deserialized_packets.len(), 1);
228237
assert!(results.new_tracer_stats_option.is_none());

core/src/banking_stage/packet_receiver.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ impl PacketReceiver {
4949
.receive_packets(
5050
recv_timeout,
5151
unprocessed_transaction_storage.max_receive_size(),
52+
|packet| packet.compute_unit_limit_above_static_builtins(),
5253
)
5354
// Consumes results if Ok, otherwise we keep the Err
5455
.map(|receive_packet_results| {

0 commit comments

Comments
 (0)