Cleanup and feature gate instruction processing (#12359)

This commit is contained in:
sakridge
2020-09-20 10:58:12 -07:00
committed by GitHub
parent 65b247a922
commit 22d8b3c3f8
5 changed files with 43 additions and 19 deletions

View File

@ -26,14 +26,14 @@ fn process_instruction(
return Err(ProgramError::InvalidAccountData); return Err(ProgramError::InvalidAccountData);
} }
let instruction = instructions::get_instruction( let instruction = instructions::load_instruction_at(
secp_instruction_index as usize, secp_instruction_index as usize,
&instruction_accounts.try_borrow_data()?, &instruction_accounts.try_borrow_data()?,
) )
.map_err(|_| ProgramError::InvalidAccountData)?; .map_err(|_| ProgramError::InvalidAccountData)?;
let current_instruction = let current_instruction =
instructions::get_current_instruction(&instruction_accounts.try_borrow_data()?); instructions::load_current_index(&instruction_accounts.try_borrow_data()?);
let my_index = instruction_data[1] as u16; let my_index = instruction_data[1] as u16;
assert_eq!(current_instruction, my_index); assert_eq!(current_instruction, my_index);

View File

@ -1982,6 +1982,8 @@ impl Bank {
&self.rent_collector, &self.rent_collector,
log_collector.clone(), log_collector.clone(),
executors.clone(), executors.clone(),
self.cluster_type(),
self.epoch(),
); );
Self::refcells_to_accounts( Self::refcells_to_accounts(

View File

@ -10,6 +10,7 @@ use solana_sdk::{
ComputeBudget, ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext, ComputeBudget, ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext,
Executor, InvokeContext, Logger, ProcessInstruction, ProcessInstructionWithContext, Executor, InvokeContext, Logger, ProcessInstruction, ProcessInstructionWithContext,
}, },
genesis_config::ClusterType,
instruction::{CompiledInstruction, InstructionError}, instruction::{CompiledInstruction, InstructionError},
message::Message, message::Message,
native_loader, native_loader,
@ -656,6 +657,7 @@ impl MessageProcessor {
/// This method calls the instruction's program entrypoint method and verifies that the result of /// This method calls the instruction's program entrypoint method and verifies that the result of
/// the call does not violate the bank's accounting rules. /// the call does not violate the bank's accounting rules.
/// The accounts are committed back to the bank only if this function returns Ok(_). /// The accounts are committed back to the bank only if this function returns Ok(_).
#[allow(clippy::too_many_arguments)]
fn execute_instruction( fn execute_instruction(
&self, &self,
message: &Message, message: &Message,
@ -666,17 +668,21 @@ impl MessageProcessor {
log_collector: Option<Rc<LogCollector>>, log_collector: Option<Rc<LogCollector>>,
executors: Rc<RefCell<Executors>>, executors: Rc<RefCell<Executors>>,
instruction_index: usize, instruction_index: usize,
cluster_type: ClusterType,
epoch: Epoch,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
// Fixup the special instructions key if present // Fixup the special instructions key if present
// before the account pre-values are taken care of // before the account pre-values are taken care of
for (i, key) in message.account_keys.iter().enumerate() { if solana_sdk::sysvar::instructions::is_enabled(epoch, cluster_type) {
if solana_sdk::sysvar::instructions::check_id(key) { for (i, key) in message.account_keys.iter().enumerate() {
let mut mut_account_ref = accounts[i].borrow_mut(); if solana_sdk::sysvar::instructions::check_id(key) {
solana_sdk::sysvar::instructions::store_current_instruction( let mut mut_account_ref = accounts[i].borrow_mut();
&mut mut_account_ref.data, solana_sdk::sysvar::instructions::store_current_index(
instruction_index as u16, &mut mut_account_ref.data,
); instruction_index as u16,
break; );
break;
}
} }
} }
@ -716,6 +722,8 @@ impl MessageProcessor {
rent_collector: &RentCollector, rent_collector: &RentCollector,
log_collector: Option<Rc<LogCollector>>, log_collector: Option<Rc<LogCollector>>,
executors: Rc<RefCell<Executors>>, executors: Rc<RefCell<Executors>>,
cluster_type: ClusterType,
epoch: Epoch,
) -> Result<(), TransactionError> { ) -> Result<(), TransactionError> {
for (instruction_index, instruction) in message.instructions.iter().enumerate() { for (instruction_index, instruction) in message.instructions.iter().enumerate() {
self.execute_instruction( self.execute_instruction(
@ -727,6 +735,8 @@ impl MessageProcessor {
log_collector.clone(), log_collector.clone(),
executors.clone(), executors.clone(),
instruction_index, instruction_index,
cluster_type,
epoch,
) )
.map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?;
} }
@ -1319,6 +1329,8 @@ mod tests {
&rent_collector, &rent_collector,
None, None,
executors.clone(), executors.clone(),
ClusterType::Development,
0,
); );
assert_eq!(result, Ok(())); assert_eq!(result, Ok(()));
assert_eq!(accounts[0].borrow().lamports, 100); assert_eq!(accounts[0].borrow().lamports, 100);
@ -1340,6 +1352,8 @@ mod tests {
&rent_collector, &rent_collector,
None, None,
executors.clone(), executors.clone(),
ClusterType::Development,
0,
); );
assert_eq!( assert_eq!(
result, result,
@ -1365,6 +1379,8 @@ mod tests {
&rent_collector, &rent_collector,
None, None,
executors, executors,
ClusterType::Development,
0,
); );
assert_eq!( assert_eq!(
result, result,
@ -1473,6 +1489,8 @@ mod tests {
&rent_collector, &rent_collector,
None, None,
executors.clone(), executors.clone(),
ClusterType::Development,
0,
); );
assert_eq!( assert_eq!(
result, result,
@ -1498,6 +1516,8 @@ mod tests {
&rent_collector, &rent_collector,
None, None,
executors.clone(), executors.clone(),
ClusterType::Development,
0,
); );
assert_eq!(result, Ok(())); assert_eq!(result, Ok(()));
@ -1520,6 +1540,8 @@ mod tests {
&rent_collector, &rent_collector,
None, None,
executors, executors,
ClusterType::Development,
0,
); );
assert_eq!(result, Ok(())); assert_eq!(result, Ok(()));
assert_eq!(accounts[0].borrow().lamports, 80); assert_eq!(accounts[0].borrow().lamports, 80);

View File

@ -47,7 +47,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) {
let serialized = message.serialize_instructions(); let serialized = message.serialize_instructions();
b.iter(|| { b.iter(|| {
for i in 0..instructions.len() { for i in 0..instructions.len() {
test::black_box(instructions::get_instruction(i, &serialized).unwrap()); test::black_box(instructions::load_instruction_at(i, &serialized).unwrap());
} }
}); });
} }
@ -58,6 +58,6 @@ fn bench_manual_instruction_deserialize_single(b: &mut Bencher) {
let message = Message::new(&instructions, None); let message = Message::new(&instructions, None);
let serialized = message.serialize_instructions(); let serialized = message.serialize_instructions();
b.iter(|| { b.iter(|| {
test::black_box(instructions::get_instruction(3, &serialized).unwrap()); test::black_box(instructions::load_instruction_at(3, &serialized).unwrap());
}); });
} }

View File

@ -7,7 +7,7 @@ use crate::sysvar::Sysvar;
pub type Instructions = Vec<Instruction>; pub type Instructions = Vec<Instruction>;
crate::declare_sysvar_id!("instructions1111111111111111111111111111111", Instructions); crate::declare_sysvar_id!("Sysvar1nstructions1111111111111111111111111", Instructions);
impl Sysvar for Instructions {} impl Sysvar for Instructions {}
@ -21,19 +21,19 @@ pub fn is_enabled(_epoch: Epoch, cluster_type: ClusterType) -> bool {
cluster_type == ClusterType::Development cluster_type == ClusterType::Development
} }
pub fn get_current_instruction(data: &[u8]) -> u16 { pub fn load_current_index(data: &[u8]) -> u16 {
let mut instr_fixed_data = [0u8; 2]; let mut instr_fixed_data = [0u8; 2];
let len = data.len(); let len = data.len();
instr_fixed_data.copy_from_slice(&data[len - 2..len]); instr_fixed_data.copy_from_slice(&data[len - 2..len]);
u16::from_le_bytes(instr_fixed_data) u16::from_le_bytes(instr_fixed_data)
} }
pub fn store_current_instruction(data: &mut [u8], instruction_index: u16) { pub fn store_current_index(data: &mut [u8], instruction_index: u16) {
let last_index = data.len() - 2; let last_index = data.len() - 2;
data[last_index..last_index + 2].copy_from_slice(&instruction_index.to_le_bytes()); data[last_index..last_index + 2].copy_from_slice(&instruction_index.to_le_bytes());
} }
pub fn get_instruction(index: usize, data: &[u8]) -> Result<Instruction, SanitizeError> { pub fn load_instruction_at(index: usize, data: &[u8]) -> Result<Instruction, SanitizeError> {
solana_sdk::message::Message::deserialize_instruction(index, data) solana_sdk::message::Message::deserialize_instruction(index, data)
} }
@ -42,10 +42,10 @@ mod tests {
use super::*; use super::*;
#[test] #[test]
fn test_get_store_instruction() { fn test_load_store_instruction() {
let mut data = [4u8; 10]; let mut data = [4u8; 10];
store_current_instruction(&mut data, 3); store_current_index(&mut data, 3);
assert_eq!(get_current_instruction(&data), 3); assert_eq!(load_current_index(&data), 3);
assert_eq!([4u8; 8], data[0..8]); assert_eq!([4u8; 8], data[0..8]);
} }
} }