Skip to content

Commit

Permalink
Record inner instruction stack height (solana-labs#28430)
Browse files Browse the repository at this point in the history
* Record inner instruction stack height

* fix sbf tests

* feedback
  • Loading branch information
jstarry authored Oct 26, 2022
1 parent 74bd87d commit 2d8665d
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 43 deletions.
19 changes: 15 additions & 4 deletions ledger/src/blockstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4406,7 +4406,9 @@ pub mod tests {
transaction_context::TransactionReturnData,
},
solana_storage_proto::convert::generated,
solana_transaction_status::{InnerInstructions, Reward, Rewards, TransactionTokenBalance},
solana_transaction_status::{
InnerInstruction, InnerInstructions, Reward, Rewards, TransactionTokenBalance,
},
std::{thread::Builder, time::Duration},
};

Expand Down Expand Up @@ -6838,7 +6840,10 @@ pub mod tests {
let post_balances_vec = vec![3, 2, 1];
let inner_instructions_vec = vec![InnerInstructions {
index: 0,
instructions: vec![CompiledInstruction::new(1, &(), vec![0])],
instructions: vec![InnerInstruction {
instruction: CompiledInstruction::new(1, &(), vec![0]),
stack_height: Some(2),
}],
}];
let log_messages_vec = vec![String::from("Test message\n")];
let pre_token_balances_vec = vec![];
Expand Down Expand Up @@ -7570,7 +7575,10 @@ pub mod tests {
}
let inner_instructions = Some(vec![InnerInstructions {
index: 0,
instructions: vec![CompiledInstruction::new(1, &(), vec![0])],
instructions: vec![InnerInstruction {
instruction: CompiledInstruction::new(1, &(), vec![0]),
stack_height: Some(2),
}],
}]);
let log_messages = Some(vec![String::from("Test message\n")]);
let pre_token_balances = Some(vec![]);
Expand Down Expand Up @@ -7687,7 +7695,10 @@ pub mod tests {
}
let inner_instructions = Some(vec![InnerInstructions {
index: 0,
instructions: vec![CompiledInstruction::new(1, &(), vec![0])],
instructions: vec![InnerInstruction {
instruction: CompiledInstruction::new(1, &(), vec![0]),
stack_height: Some(2),
}],
}]);
let log_messages = Some(vec![String::from("Test message\n")]);
let pre_token_balances = Some(vec![]);
Expand Down
25 changes: 17 additions & 8 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use {
solana_program_runtime::{compute_budget::ComputeBudget, timings::ExecuteTimings},
solana_runtime::{
bank::{
DurableNonceFee, TransactionBalancesSet, TransactionExecutionDetails,
DurableNonceFee, InnerInstruction, TransactionBalancesSet, TransactionExecutionDetails,
TransactionExecutionResult, TransactionResults,
},
loader_utils::{
Expand All @@ -33,7 +33,6 @@ use {
feature_set::FeatureSet,
fee::FeeStructure,
fee_calculator::FeeRateGovernor,
instruction::CompiledInstruction,
loader_instruction,
message::{v0::LoadedAddresses, SanitizedMessage},
rent::Rent,
Expand Down Expand Up @@ -374,7 +373,7 @@ fn process_transaction_and_record_inner(
tx: Transaction,
) -> (
Result<(), TransactionError>,
Vec<Vec<CompiledInstruction>>,
Vec<Vec<InnerInstruction>>,
Vec<String>,
) {
let signature = tx.signatures.get(0).unwrap().clone();
Expand Down Expand Up @@ -491,7 +490,13 @@ fn execute_transactions(
.enumerate()
.map(|(index, instructions)| InnerInstructions {
index: index as u8,
instructions,
instructions: instructions
.into_iter()
.map(|ix| solana_transaction_status::InnerInstruction {
instruction: ix.instruction,
stack_height: Some(u32::from(ix.stack_height)),
})
.collect(),
})
.filter(|i| !i.instructions.is_empty())
.collect()
Expand Down Expand Up @@ -1110,7 +1115,8 @@ fn test_program_sbf_invoke_sanity() {

let invoked_programs: Vec<Pubkey> = inner_instructions[0]
.iter()
.map(|ix| message.account_keys[ix.program_id_index as usize].clone())
.map(|ix| &message.account_keys[ix.instruction.program_id_index as usize])
.cloned()
.collect();
let expected_invoked_programs = match program.0 {
Languages::C => vec![
Expand Down Expand Up @@ -1165,7 +1171,8 @@ fn test_program_sbf_invoke_sanity() {
assert_eq!(invoked_programs, expected_invoked_programs);
let no_invoked_programs: Vec<Pubkey> = inner_instructions[1]
.iter()
.map(|ix| message.account_keys[ix.program_id_index as usize].clone())
.map(|ix| &message.account_keys[ix.instruction.program_id_index as usize])
.cloned()
.collect();
assert_eq!(no_invoked_programs.len(), 0);

Expand Down Expand Up @@ -1195,7 +1202,8 @@ fn test_program_sbf_invoke_sanity() {
process_transaction_and_record_inner(&bank, tx);
let invoked_programs: Vec<Pubkey> = inner_instructions[0]
.iter()
.map(|ix| message.account_keys[ix.program_id_index as usize].clone())
.map(|ix| &message.account_keys[ix.instruction.program_id_index as usize])
.cloned()
.collect();
assert_eq!(result, Err(expected_error));
assert_eq!(invoked_programs, expected_invoked_programs);
Expand Down Expand Up @@ -1411,7 +1419,8 @@ fn test_program_sbf_invoke_sanity() {
process_transaction_and_record_inner(&bank, tx);
let invoked_programs: Vec<Pubkey> = inner_instructions[0]
.iter()
.map(|ix| message.account_keys[ix.program_id_index as usize].clone())
.map(|ix| &message.account_keys[ix.instruction.program_id_index as usize])
.cloned()
.collect();
assert_eq!(invoked_programs, vec![system_program::id()]);
assert_eq!(
Expand Down
1 change: 1 addition & 0 deletions rpc-client/src/mock_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ impl RpcSender for MockSender {
program_id_index: 2,
accounts: vec![0, 1],
data: "3Bxs49DitAvXtoDR".to_string(),
stack_height: None,
}],
address_table_lookups: None,
})
Expand Down
10 changes: 8 additions & 2 deletions rpc/src/transaction_status_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use {
},
solana_runtime::bank::{DurableNonceFee, TransactionExecutionDetails},
solana_transaction_status::{
extract_and_fmt_memos, InnerInstructions, Reward, TransactionStatusMeta,
extract_and_fmt_memos, InnerInstruction, InnerInstructions, Reward, TransactionStatusMeta,
},
std::{
sync::{
Expand Down Expand Up @@ -128,7 +128,13 @@ impl TransactionStatusService {
.enumerate()
.map(|(index, instructions)| InnerInstructions {
index: index as u8,
instructions,
instructions: instructions
.into_iter()
.map(|info| InnerInstruction {
instruction: info.instruction,
stack_height: Some(u32::from(info.stack_height)),
})
.collect(),
})
.filter(|i| !i.instructions.is_empty())
.collect()
Expand Down
42 changes: 34 additions & 8 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,15 @@ pub type TransactionBalances = Vec<Vec<u64>>;

/// An ordered list of compiled instructions that were invoked during a
/// transaction instruction
pub type InnerInstructions = Vec<CompiledInstruction>;
pub type InnerInstructions = Vec<InnerInstruction>;

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct InnerInstruction {
pub instruction: CompiledInstruction,
/// Invocation stack height of this instruction. Instruction stack height
/// starts at 1 for transaction instructions.
pub stack_height: u8,
}

/// A list of compiled instructions that were invoked during each instruction of
/// a transaction
Expand All @@ -511,10 +519,12 @@ pub fn inner_instructions_list_from_instruction_trace(
if let Ok(instruction_context) =
transaction_context.get_instruction_context_at_index_in_trace(index_in_trace)
{
if instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT {
let stack_height = instruction_context.get_stack_height();
if 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(
let stack_height = u8::try_from(stack_height).unwrap_or(u8::MAX);
let instruction = CompiledInstruction::new_from_raw_parts(
instruction_context
.get_index_of_program_account_in_transaction(
instruction_context
Expand All @@ -532,7 +542,11 @@ pub fn inner_instructions_list_from_instruction_trace(
.unwrap_or_default() as u8
})
.collect(),
));
);
inner_instructions.push(InnerInstruction {
instruction,
stack_height,
});
} else {
debug_assert!(false);
}
Expand Down Expand Up @@ -19269,12 +19283,24 @@ pub(crate) mod tests {
assert_eq!(
inner_instructions,
vec![
vec![CompiledInstruction::new_from_raw_parts(0, vec![1], vec![])],
vec![InnerInstruction {
instruction: CompiledInstruction::new_from_raw_parts(0, vec![1], vec![]),
stack_height: 2,
}],
vec![],
vec![
CompiledInstruction::new_from_raw_parts(0, vec![4], vec![]),
CompiledInstruction::new_from_raw_parts(0, vec![5], vec![]),
CompiledInstruction::new_from_raw_parts(0, vec![6], vec![])
InnerInstruction {
instruction: CompiledInstruction::new_from_raw_parts(0, vec![4], vec![]),
stack_height: 2,
},
InnerInstruction {
instruction: CompiledInstruction::new_from_raw_parts(0, vec![5], vec![]),
stack_height: 3,
},
InnerInstruction {
instruction: CompiledInstruction::new_from_raw_parts(0, vec![6], vec![]),
stack_height: 2,
},
]
]
);
Expand Down
13 changes: 12 additions & 1 deletion storage-proto/proto/confirmed_block.proto
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,18 @@ message TransactionError {

message InnerInstructions {
uint32 index = 1;
repeated CompiledInstruction instructions = 2;
repeated InnerInstruction instructions = 2;
}

message InnerInstruction {
uint32 program_id_index = 1;
bytes accounts = 2;
bytes data = 3;

// Invocation stack height of an inner instruction.
// Available since Solana v1.14.6
// Set to `None` for txs executed on earlier versions.
optional uint32 stack_height = 4;
}

message CompiledInstruction {
Expand Down
30 changes: 27 additions & 3 deletions storage-proto/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use {
transaction_context::TransactionReturnData,
},
solana_transaction_status::{
ConfirmedBlock, InnerInstructions, Reward, RewardType, TransactionByAddrInfo,
TransactionStatusMeta, TransactionTokenBalance, TransactionWithStatusMeta,
VersionedConfirmedBlock, VersionedTransactionWithStatusMeta,
ConfirmedBlock, InnerInstruction, InnerInstructions, Reward, RewardType,
TransactionByAddrInfo, TransactionStatusMeta, TransactionTokenBalance,
TransactionWithStatusMeta, VersionedConfirmedBlock, VersionedTransactionWithStatusMeta,
},
std::{
convert::{TryFrom, TryInto},
Expand Down Expand Up @@ -647,6 +647,30 @@ impl From<generated::CompiledInstruction> for CompiledInstruction {
}
}

impl From<InnerInstruction> for generated::InnerInstruction {
fn from(value: InnerInstruction) -> Self {
Self {
program_id_index: value.instruction.program_id_index as u32,
accounts: value.instruction.accounts,
data: value.instruction.data,
stack_height: value.stack_height,
}
}
}

impl From<generated::InnerInstruction> for InnerInstruction {
fn from(value: generated::InnerInstruction) -> Self {
Self {
instruction: CompiledInstruction {
program_id_index: value.program_id_index as u8,
accounts: value.accounts,
data: value.data,
},
stack_height: value.stack_height,
}
}
}

impl TryFrom<tx_by_addr::TransactionError> for TransactionError {
type Error = &'static str;

Expand Down
Loading

0 comments on commit 2d8665d

Please sign in to comment.