Skip to content

Commit

Permalink
Removes instruction_trace from ExecutionRecord.
Browse files Browse the repository at this point in the history
  • Loading branch information
Lichtso committed Aug 16, 2022
1 parent e0b82e8 commit 5fcf423
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 71 deletions.
105 changes: 55 additions & 50 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ use {
TransactionVerificationMode, VersionedTransaction, MAX_TX_ACCOUNT_LOCKS,
},
transaction_context::{
ExecutionRecord, InstructionContext, TransactionAccount, TransactionContext,
TransactionReturnData,
ExecutionRecord, TransactionAccount, TransactionContext, TransactionReturnData,
},
},
solana_stake_program::stake_state::{
Expand Down Expand Up @@ -743,39 +742,45 @@ pub type InnerInstructions = Vec<CompiledInstruction>;
/// a transaction
pub type InnerInstructionsList = Vec<InnerInstructions>;

/// Convert from an &[InstructionContext] to InnerInstructionsList
/// Extract the InnerInstructionsList from a TransactionContext
pub fn inner_instructions_list_from_instruction_trace(
instruction_trace: &[InstructionContext],
transaction_context: &TransactionContext,
) -> InnerInstructionsList {
debug_assert!(instruction_trace
.first()
debug_assert!(transaction_context
.get_instruction_context_at_index(0)
.map(|instruction_context| instruction_context.get_stack_height()
== TRANSACTION_LEVEL_STACK_HEIGHT)
.unwrap_or(true));
let mut outer_instructions = Vec::new();
for instruction_context in instruction_trace.iter() {
if instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT {
outer_instructions.push(Vec::new());
} else if let Some(inner_instructions) = outer_instructions.last_mut() {
inner_instructions.push(CompiledInstruction::new_from_raw_parts(
instruction_context
.get_index_of_program_account_in_transaction(
instruction_context
.get_number_of_program_accounts()
.saturating_sub(1),
)
.unwrap_or_default() as u8,
instruction_context.get_instruction_data().to_vec(),
(0..instruction_context.get_number_of_instruction_accounts())
.map(|instruction_account_index| {
instruction_context
.get_index_of_instruction_account_in_transaction(
instruction_account_index,
)
.unwrap_or_default() as u8
})
.collect(),
));
for index_in_trace in 0..transaction_context.get_instruction_trace_length() {
if let Ok(instruction_context) =
transaction_context.get_instruction_context_at_index(index_in_trace)
{
if instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT {
outer_instructions.push(Vec::new());
} else if let Some(inner_instructions) = outer_instructions.last_mut() {
inner_instructions.push(CompiledInstruction::new_from_raw_parts(
instruction_context
.get_index_of_program_account_in_transaction(
instruction_context
.get_number_of_program_accounts()
.saturating_sub(1),
)
.unwrap_or_default() as u8,
instruction_context.get_instruction_data().to_vec(),
(0..instruction_context.get_number_of_instruction_accounts())
.map(|instruction_account_index| {
instruction_context
.get_index_of_instruction_account_in_transaction(
instruction_account_index,
)
.unwrap_or_default() as u8
})
.collect(),
));
} else {
debug_assert!(false);
}
} else {
debug_assert!(false);
}
Expand Down Expand Up @@ -4404,9 +4409,16 @@ impl Bank {
.ok()
});

let inner_instructions = if enable_cpi_recording {
Some(inner_instructions_list_from_instruction_trace(
&transaction_context,
))
} else {
None
};

let ExecutionRecord {
accounts,
instruction_trace,
mut return_data,
changed_account_count,
total_size_of_all_accounts,
Expand Down Expand Up @@ -4434,14 +4446,6 @@ impl Bank {
accounts_data_len_delta = status.as_ref().map_or(0, |_| accounts_resize_delta);
}

let inner_instructions = if enable_cpi_recording {
Some(inner_instructions_list_from_instruction_trace(
&instruction_trace,
))
} else {
None
};

let return_data = if enable_return_data_recording {
if let Some(end_index) = return_data.data.iter().rposition(|&x| x != 0) {
let end_index = end_index.saturating_add(1);
Expand Down Expand Up @@ -8054,7 +8058,6 @@ pub(crate) mod tests {
system_program,
timing::duration_as_s,
transaction::MAX_TX_ACCOUNT_LOCKS,
transaction_context::InstructionContext,
},
solana_vote_program::{
vote_instruction,
Expand Down Expand Up @@ -18891,17 +18894,19 @@ pub(crate) mod tests {

#[test]
fn test_inner_instructions_list_from_instruction_trace() {
let instruction_trace = vec![
InstructionContext::new(0, 0, &[], &[], &[0]),
InstructionContext::new(1, 0, &[], &[], &[1]),
InstructionContext::new(0, 0, &[], &[], &[2]),
InstructionContext::new(0, 0, &[], &[], &[3]),
InstructionContext::new(1, 0, &[], &[], &[4]),
InstructionContext::new(2, 0, &[], &[], &[5]),
InstructionContext::new(1, 0, &[], &[], &[6]),
];

let inner_instructions = inner_instructions_list_from_instruction_trace(&instruction_trace);
let mut transaction_context = TransactionContext::new(vec![], None, 3, 3);
for (index_in_trace, stack_height) in [1, 2, 1, 1, 2, 3, 2].into_iter().enumerate() {
while stack_height <= transaction_context.get_instruction_context_stack_height() {
transaction_context.pop().unwrap();
}
if stack_height > transaction_context.get_instruction_context_stack_height() {
transaction_context
.push(&[], &[], &[index_in_trace as u8])
.unwrap();
}
}
let inner_instructions =
inner_instructions_list_from_instruction_trace(&transaction_context);

assert_eq!(
inner_instructions,
Expand Down
39 changes: 18 additions & 21 deletions sdk/src/transaction_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,26 +236,27 @@ impl TransactionContext {
return Err(InstructionError::CallDepth);
}
// Verify (before we pop) that the total sum of all lamports in this instruction did not change
let detected_an_unbalanced_instruction = if self
.is_early_verification_of_account_modifications_enabled()
{
self.get_current_instruction_context()
.and_then(|instruction_context| {
// Verify all executable accounts have no outstanding refs
for account_index in instruction_context.program_accounts.iter() {
self.get_account_at_index(*account_index)?
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
}
self.instruction_accounts_lamport_sum(instruction_context.instruction_accounts.iter())
let detected_an_unbalanced_instruction =
if self.is_early_verification_of_account_modifications_enabled() {
self.get_current_instruction_context()
.and_then(|instruction_context| {
// Verify all executable accounts have no outstanding refs
for account_index in instruction_context.program_accounts.iter() {
self.get_account_at_index(*account_index)?
.try_borrow_mut()
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
}
self.instruction_accounts_lamport_sum(
instruction_context.instruction_accounts.iter(),
)
.map(|instruction_accounts_lamport_sum| {
instruction_context.instruction_accounts_lamport_sum
!= instruction_accounts_lamport_sum
})
})
} else {
Ok(false)
};
})
} else {
Ok(false)
};
// Always pop, even if we `detected_an_unbalanced_instruction`
self.instruction_stack.pop();
if detected_an_unbalanced_instruction? {
Expand Down Expand Up @@ -292,9 +293,7 @@ impl TransactionContext {
return Ok(0);
}
let mut instruction_accounts_lamport_sum: u128 = 0;
for (instruction_account_index, instruction_account) in
instruction_accounts.enumerate()
{
for (instruction_account_index, instruction_account) in instruction_accounts.enumerate() {
if instruction_account_index != instruction_account.index_in_callee {
continue; // Skip duplicate account
}
Expand Down Expand Up @@ -894,7 +893,6 @@ impl<'a> BorrowedAccount<'a> {
/// Everything that needs to be recorded from a TransactionContext after execution
pub struct ExecutionRecord {
pub accounts: Vec<TransactionAccount>,
pub instruction_trace: Vec<InstructionContext>,
pub return_data: TransactionReturnData,
pub changed_account_count: u64,
pub total_size_of_all_accounts: u64,
Expand Down Expand Up @@ -937,7 +935,6 @@ impl From<TransactionContext> for ExecutionRecord {
.map(|account| account.into_inner()),
)
.collect(),
instruction_trace: context.instruction_trace,
return_data: context.return_data,
changed_account_count,
total_size_of_all_accounts,
Expand Down

0 comments on commit 5fcf423

Please sign in to comment.