Skip to content

Commit

Permalink
remove inner "details" struct, flatten into single structure. Fixed a…
Browse files Browse the repository at this point in the history 
… bug that misses counting non-builtins, that broke tests
  • Loading branch information
@tao-stones
tao-stones committed Aug 8, 2024
1 parent 0b14274 commit 787326d
Show file tree
Hide file tree
Showing 3 changed files with 182 additions and 191 deletions.
28 changes: 11 additions & 17 deletions runtime-transaction/src/builtin_instruction_details.rs
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
use {
crate::instruction_details::InstructionDetails,
solana_builtins_default_costs::BUILTIN_INSTRUCTION_COSTS,
solana_sdk::{
instruction::CompiledInstruction, pubkey::Pubkey, saturating_add_assign,
transaction::Result,
},
};

#[cfg_attr(test, derive(Clone, Eq, PartialEq))]
#[derive(Default, Debug)]
pub(crate) struct BuiltinInstructionDetails {
// builtin instruction details
pub sum_builtin_compute_units: u32,
pub count_builtin_instructions: u32,
pub count_non_builtin_instructions: u32,
}

impl BuiltinInstructionDetails {
pub fn process_instruction<'a>(
impl InstructionDetails {
pub fn process_builtin_instruction<'a>(
&mut self,
program_id: &'a Pubkey,
_instruction: &'a CompiledInstruction, // reserved to identify builtin cost by instruction
Expand All @@ -41,37 +33,39 @@ mod tests {
use super::*;

#[test]
fn test_process_instruction() {
let mut builtin_instruction_details = BuiltinInstructionDetails::default();
fn test_process_builtin_instruction() {
let mut builtin_instruction_details = InstructionDetails::default();
let dummy_ix = CompiledInstruction::new_from_raw_parts(0, vec![], vec![]);

// process all builtin with default costs
for (pubkey, cost) in BUILTIN_INSTRUCTION_COSTS.iter() {
let expected_value = BuiltinInstructionDetails {
let expected_value = InstructionDetails {
sum_builtin_compute_units: builtin_instruction_details.sum_builtin_compute_units
+ *cost as u32,
count_builtin_instructions: builtin_instruction_details.count_builtin_instructions
+ 1,
count_non_builtin_instructions: 0,
..InstructionDetails::default()
};

assert!(builtin_instruction_details
.process_instruction(pubkey, &dummy_ix)
.process_builtin_instruction(pubkey, &dummy_ix)
.is_ok());
assert_eq!(builtin_instruction_details, expected_value);
}

// continue process non-builtin instruction
let expected_value = BuiltinInstructionDetails {
let expected_value = InstructionDetails {
sum_builtin_compute_units: builtin_instruction_details.sum_builtin_compute_units,
count_builtin_instructions: builtin_instruction_details.count_builtin_instructions,
count_non_builtin_instructions: builtin_instruction_details
.count_non_builtin_instructions
+ 1,
..InstructionDetails::default()
};

assert!(builtin_instruction_details
.process_instruction(&Pubkey::new_unique(), &dummy_ix)
.process_builtin_instruction(&Pubkey::new_unique(), &dummy_ix)
.is_ok());
assert_eq!(builtin_instruction_details, expected_value);
}
Expand Down
105 changes: 56 additions & 49 deletions runtime-transaction/src/compute_budget_instruction_details.rs
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,17 @@
use solana_sdk::{
borsh1::try_from_slice_unchecked,
compute_budget::{self, ComputeBudgetInstruction},
instruction::{CompiledInstruction, InstructionError},
pubkey::Pubkey,
saturating_add_assign,
transaction::{Result, TransactionError},
use {
crate::instruction_details::InstructionDetails,
solana_sdk::{
borsh1::try_from_slice_unchecked,
compute_budget::{self, ComputeBudgetInstruction},
instruction::{CompiledInstruction, InstructionError},
pubkey::Pubkey,
saturating_add_assign,
transaction::{Result, TransactionError},
},
};

#[cfg_attr(test, derive(Eq, PartialEq))]
#[derive(Default, Debug)]
pub(crate) struct ComputeBudgetInstructionDetails {
// compute-budget instruction details:
// the first field in tuple is instruction index, second field is the unsanitized value set by user
pub requested_compute_unit_limit: Option<(u8, u32)>,
pub requested_compute_unit_price: Option<(u8, u64)>,
pub requested_heap_size: Option<(u8, u32)>,
pub requested_loaded_accounts_data_size_limit: Option<(u8, u32)>,
pub count_compute_budget_instructions: u32,
}

impl ComputeBudgetInstructionDetails {
pub fn process_instruction<'a>(
impl InstructionDetails {
pub fn process_compute_budget_instruction<'a>(
&mut self,
index: u8,
program_id: &'a Pubkey,
Expand Down Expand Up @@ -87,10 +78,10 @@ mod test {
}

#[test]
fn test_process_instruction_request_heap() {
fn test_process_compute_budget_instruction_request_heap() {
let mut index = 0;
let mut expected_details = ComputeBudgetInstructionDetails::default();
let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default();
let mut expected_details = InstructionDetails::default();
let mut compute_budget_instruction_details = InstructionDetails::default();

// irrelevant instruction makes no change
index += 1;
Expand All @@ -99,7 +90,7 @@ mod test {
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -112,7 +103,7 @@ mod test {
expected_details.requested_heap_size = Some((index, 40 * 1024));
expected_details.count_compute_budget_instructions = 1;
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -124,7 +115,11 @@ mod test {
ComputeBudgetInstruction::request_heap_frame(50 * 1024),
);
assert_eq!(
compute_budget_instruction_details.process_instruction(index, &program_id, &ix),
compute_budget_instruction_details.process_compute_budget_instruction(
index,
&program_id,
&ix
),
expected_err
);
assert_eq!(compute_budget_instruction_details, expected_details);
Expand All @@ -136,24 +131,24 @@ mod test {
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);
}

#[test]
fn test_process_instruction_compute_unit_limit() {
fn test_process_compute_budget_instruction_compute_unit_limit() {
let mut index = 0;
let mut expected_details = ComputeBudgetInstructionDetails::default();
let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default();
let mut expected_details = InstructionDetails::default();
let mut compute_budget_instruction_details = InstructionDetails::default();

// irrelevant instruction makes no change
let (program_id, ix) = setup_test_instruction(
index,
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -166,7 +161,7 @@ mod test {
expected_details.requested_compute_unit_limit = Some((index, u32::MAX));
expected_details.count_compute_budget_instructions = 1;
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -178,7 +173,11 @@ mod test {
ComputeBudgetInstruction::set_compute_unit_limit(MAX_COMPUTE_UNIT_LIMIT),
);
assert_eq!(
compute_budget_instruction_details.process_instruction(index, &program_id, &ix),
compute_budget_instruction_details.process_compute_budget_instruction(
index,
&program_id,
&ix
),
expected_err
);
assert_eq!(compute_budget_instruction_details, expected_details);
Expand All @@ -190,24 +189,24 @@ mod test {
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);
}

#[test]
fn test_process_instruction_compute_unit_price() {
fn test_process_compute_budget_instruction_compute_unit_price() {
let mut index = 0;
let mut expected_details = ComputeBudgetInstructionDetails::default();
let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default();
let mut expected_details = InstructionDetails::default();
let mut compute_budget_instruction_details = InstructionDetails::default();

// irrelevant instruction makes no change
let (program_id, ix) = setup_test_instruction(
index,
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -220,7 +219,7 @@ mod test {
expected_details.requested_compute_unit_price = Some((index, u64::MAX));
expected_details.count_compute_budget_instructions = 1;
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -230,7 +229,11 @@ mod test {
let (program_id, ix) =
setup_test_instruction(index, ComputeBudgetInstruction::set_compute_unit_price(0));
assert_eq!(
compute_budget_instruction_details.process_instruction(index, &program_id, &ix),
compute_budget_instruction_details.process_compute_budget_instruction(
index,
&program_id,
&ix
),
expected_err
);
assert_eq!(compute_budget_instruction_details, expected_details);
Expand All @@ -242,24 +245,24 @@ mod test {
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);
}

#[test]
fn test_process_instruction_loaded_accounts_data_size_limit() {
fn test_process_compute_budget_instruction_loaded_accounts_data_size_limit() {
let mut index = 0;
let mut expected_details = ComputeBudgetInstructionDetails::default();
let mut compute_budget_instruction_details = ComputeBudgetInstructionDetails::default();
let mut expected_details = InstructionDetails::default();
let mut compute_budget_instruction_details = InstructionDetails::default();

// irrelevant instruction makes no change
let (program_id, ix) = setup_test_instruction(
index,
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -272,7 +275,7 @@ mod test {
expected_details.requested_loaded_accounts_data_size_limit = Some((index, u32::MAX));
expected_details.count_compute_budget_instructions = 1;
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);

Expand All @@ -284,7 +287,11 @@ mod test {
ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(0),
);
assert_eq!(
compute_budget_instruction_details.process_instruction(index, &program_id, &ix),
compute_budget_instruction_details.process_compute_budget_instruction(
index,
&program_id,
&ix
),
expected_err
);
assert_eq!(compute_budget_instruction_details, expected_details);
Expand All @@ -296,7 +303,7 @@ mod test {
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
);
assert!(compute_budget_instruction_details
.process_instruction(index, &program_id, &ix)
.process_compute_budget_instruction(index, &program_id, &ix)
.is_ok());
assert_eq!(compute_budget_instruction_details, expected_details);
}
Expand Down
Loading

0 comments on commit 787326d

Please sign in to comment.