Skip to content

Commit

Permalink
split filters for cleaner code, adjustred inline-ness
Browse files Browse the repository at this point in the history
  • Loading branch information
tao-stones committed Dec 4, 2024
1 parent 0a9f3a2 commit 36a2346
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 126 deletions.
1 change: 1 addition & 0 deletions builtins-default-costs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ pub fn get_builtin_instruction_cost<'a>(
.map(|builtin_cost| builtin_cost.native_cost)
}

#[inline]
pub fn is_builtin_program(program_id: &Pubkey) -> bool {
BUILTIN_INSTRUCTION_COSTS.contains_key(program_id)
}
Expand Down
105 changes: 105 additions & 0 deletions runtime-transaction/src/builtin_programs_filter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
use {
agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE,
solana_builtins_default_costs::{is_builtin_program, MAYBE_BUILTIN_KEY},
solana_sdk::pubkey::Pubkey,
};

#[derive(Clone, Copy, Debug, PartialEq)]
pub(crate) enum ProgramKind {
NotBuiltin,
Builtin,
}

pub(crate) struct BuiltinProgramsFilter {
// array of slots for all possible static and sanitized program_id_index,
// each slot indicates if a program_id_index has not been checked (eg, None),
// or already checked with result (eg, Some(ProgramKind)) that can be reused.
program_kind: [Option<ProgramKind>; FILTER_SIZE as usize],
}

impl BuiltinProgramsFilter {
pub(crate) fn new() -> Self {
BuiltinProgramsFilter {
program_kind: [None; FILTER_SIZE as usize],
}
}

pub(crate) fn get_program_kind(&mut self, index: usize, program_id: &Pubkey) -> ProgramKind {
*self
.program_kind
.get_mut(index)
.expect("program id index is sanitized")
.get_or_insert_with(|| Self::check_program_kind(program_id))
}

#[inline]
fn check_program_kind(program_id: &Pubkey) -> ProgramKind {
if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] {
return ProgramKind::NotBuiltin;
}

if is_builtin_program(program_id) {
ProgramKind::Builtin
} else {
ProgramKind::NotBuiltin
}
}
}

#[cfg(test)]
mod test {
use super::*;

const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111";

#[test]
fn get_program_kind() {
let mut test_store = BuiltinProgramsFilter::new();
let mut index = 9;

// initial state is Unchecked
assert!(test_store.program_kind[index].is_none());

// non builtin returns None
assert_eq!(
test_store.get_program_kind(index, &DUMMY_PROGRAM_ID.parse().unwrap()),
ProgramKind::NotBuiltin
);
// but its state is now checked (eg, Some(...))
assert_eq!(
test_store.program_kind[index],
Some(ProgramKind::NotBuiltin)
);
// lookup same `index` will return cached data, will not lookup `program_id`
// again
assert_eq!(
test_store.get_program_kind(index, &solana_sdk::loader_v4::id()),
ProgramKind::NotBuiltin
);

// not-migrating builtin
index += 1;
assert_eq!(
test_store.get_program_kind(index, &solana_sdk::loader_v4::id()),
ProgramKind::Builtin,
);

// compute-budget
index += 1;
assert_eq!(
test_store.get_program_kind(index, &solana_sdk::compute_budget::id()),
ProgramKind::Builtin,
);
}

#[test]
#[should_panic(expected = "program id index is sanitized")]
fn test_get_program_kind_out_of_bound_index() {
let mut test_store = BuiltinProgramsFilter::new();
assert_eq!(
test_store
.get_program_kind(FILTER_SIZE as usize + 1, &DUMMY_PROGRAM_ID.parse().unwrap(),),
ProgramKind::NotBuiltin
);
}
}
9 changes: 6 additions & 3 deletions runtime-transaction/src/compute_budget_instruction_details.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use {
crate::compute_budget_program_id_filter::{ComputeBudgetProgramIdFilter, ProgramKind},
crate::{
builtin_programs_filter::{BuiltinProgramsFilter, ProgramKind},
compute_budget_program_id_filter::ComputeBudgetProgramIdFilter,
},
solana_compute_budget::compute_budget_limits::*,
solana_sdk::{
borsh1::try_from_slice_unchecked,
Expand Down Expand Up @@ -52,10 +55,11 @@ impl ComputeBudgetInstructionDetails {
.requested_compute_unit_limit
.is_none()
{
let mut filter = BuiltinProgramsFilter::new();
// reiterate to collect builtin details
for (program_id, instruction) in instructions {
match filter.get_program_kind(instruction.program_id_index as usize, program_id) {
ProgramKind::Builtin { .. } => {
ProgramKind::Builtin => {
saturating_add_assign!(
compute_budget_instruction_details.num_builtin_instructions,
1
Expand Down Expand Up @@ -169,7 +173,6 @@ impl ComputeBudgetInstructionDetails {
(MIN_HEAP_FRAME_BYTES..=MAX_HEAP_FRAME_BYTES).contains(&bytes) && bytes % 1024 == 0
}

#[inline]
fn calculate_default_compute_unit_limit(&self, feature_set: &FeatureSet) -> u32 {
if feature_set.is_active(&feature_set::reserve_minimal_cus_for_builtin_instructions::id()) {
u32::from(self.num_builtin_instructions)
Expand Down
130 changes: 7 additions & 123 deletions runtime-transaction/src/compute_budget_program_id_filter.rs
Original file line number Diff line number Diff line change
@@ -1,152 +1,36 @@
// static account keys has max
use {
agave_transaction_view::static_account_keys_frame::MAX_STATIC_ACCOUNTS_PER_PACKET as FILTER_SIZE,
solana_builtins_default_costs::{is_builtin_program, MAYBE_BUILTIN_KEY},
solana_sdk::pubkey::Pubkey,
solana_builtins_default_costs::MAYBE_BUILTIN_KEY, solana_sdk::pubkey::Pubkey,
};

#[derive(Clone, Copy, Debug, PartialEq)]
pub(crate) enum ProgramKind {
NotBuiltin,
Builtin { is_compute_budget: bool },
}

pub(crate) struct ComputeBudgetProgramIdFilter {
// array of slots for all possible static and sanitized program_id_index,
// each slot indicates if a program_id_index has not been checked (eg, None),
// or already checked with result (eg, Some(ProgramKind)) that can be reused.
program_kind: [Option<ProgramKind>; FILTER_SIZE as usize],
// or already checked with result (eg, Some(result)) that can be reused.
flags: [Option<bool>; FILTER_SIZE as usize],
}

impl ComputeBudgetProgramIdFilter {
pub(crate) fn new() -> Self {
ComputeBudgetProgramIdFilter {
program_kind: [None; FILTER_SIZE as usize],
flags: [None; FILTER_SIZE as usize],
}
}

#[inline]
pub(crate) fn is_compute_budget_program(&mut self, index: usize, program_id: &Pubkey) -> bool {
// Access the program kind at the given index, panic if index is invalid.
match self
.program_kind
.get(index)
.expect("program id index is sanitized")
{
// If the program kind is already set, check if it matches the target ProgramKind.
Some(program_kind) => {
*program_kind
== ProgramKind::Builtin {
is_compute_budget: true,
}
}
// If the program kind is not set, calculate it, store it, and then check the condition.
None => {
let is_compute_budget = Self::check_compute_bugdet_program_id(program_id);

if is_compute_budget {
self.program_kind[index] = Some(ProgramKind::Builtin {
is_compute_budget: true,
});
}

is_compute_budget
}
}
}

#[inline]
pub(crate) fn get_program_kind(&mut self, index: usize, program_id: &Pubkey) -> ProgramKind {
*self
.program_kind
.flags
.get_mut(index)
.expect("program id index is sanitized")
.get_or_insert_with(|| Self::check_program_kind(program_id))
.get_or_insert_with(|| Self::check_program_id(program_id))
}

#[inline]
fn check_compute_budget_program_id(program_id: &Pubkey) -> bool {
fn check_program_id(program_id: &Pubkey) -> bool {
if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] {
return false;
}

solana_sdk::compute_budget::check_id(program_id)
}

#[inline]
fn check_program_kind(program_id: &Pubkey) -> ProgramKind {
if !MAYBE_BUILTIN_KEY[program_id.as_ref()[0] as usize] {
return ProgramKind::NotBuiltin;
}

if is_builtin_program(program_id) {
ProgramKind::Builtin {
is_compute_budget: solana_sdk::compute_budget::check_id(program_id),
}
} else {
ProgramKind::NotBuiltin
}
}
}

#[cfg(test)]
mod test {
use super::*;

const DUMMY_PROGRAM_ID: &str = "dummmy1111111111111111111111111111111111111";

#[test]
fn get_program_kind() {
let mut test_store = ComputeBudgetProgramIdFilter::new();
let mut index = 9;

// initial state is Unchecked
assert!(test_store.program_kind[index].is_none());

// non builtin returns None
assert_eq!(
test_store.get_program_kind(index, &DUMMY_PROGRAM_ID.parse().unwrap()),
ProgramKind::NotBuiltin
);
// but its state is now checked (eg, Some(...))
assert_eq!(
test_store.program_kind[index],
Some(ProgramKind::NotBuiltin)
);
// lookup same `index` will return cached data, will not lookup `program_id`
// again
assert_eq!(
test_store.get_program_kind(index, &solana_sdk::loader_v4::id()),
ProgramKind::NotBuiltin
);

// not-migrating builtin
index += 1;
assert_eq!(
test_store.get_program_kind(index, &solana_sdk::loader_v4::id()),
ProgramKind::Builtin {
is_compute_budget: false
}
);

// compute-budget
index += 1;
assert_eq!(
test_store.get_program_kind(index, &solana_sdk::compute_budget::id()),
ProgramKind::Builtin {
is_compute_budget: true
}
);
}

#[test]
#[should_panic(expected = "program id index is sanitized")]
fn test_get_program_kind_out_of_bound_index() {
let mut test_store = ComputeBudgetProgramIdFilter::new();
assert_eq!(
test_store
.get_program_kind(FILTER_SIZE as usize + 1, &DUMMY_PROGRAM_ID.parse().unwrap(),),
ProgramKind::NotBuiltin
);
}
}
1 change: 1 addition & 0 deletions runtime-transaction/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![cfg_attr(feature = "frozen-abi", feature(min_specialization))]
#![allow(clippy::arithmetic_side_effects)]

mod builtin_programs_filter;
pub mod compute_budget_instruction_details;
mod compute_budget_program_id_filter;
pub mod instructions_processor;
Expand Down

0 comments on commit 36a2346

Please sign in to comment.