From 22d8b3c3f88736c90bad713d05fc75bcac76f9f6 Mon Sep 17 00:00:00 2001 From: sakridge Date: Sun, 20 Sep 2020 10:58:12 -0700 Subject: [PATCH] Cleanup and feature gate instruction processing (#12359) --- .../rust/instruction_introspection/src/lib.rs | 4 +- runtime/src/bank.rs | 2 + runtime/src/message_processor.rs | 38 +++++++++++++++---- sdk/benches/serialize_instructions.rs | 4 +- sdk/src/sysvar/instructions.rs | 14 +++---- 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/programs/bpf/rust/instruction_introspection/src/lib.rs b/programs/bpf/rust/instruction_introspection/src/lib.rs index c82efbdf2c..a4ec1ffd2a 100644 --- a/programs/bpf/rust/instruction_introspection/src/lib.rs +++ b/programs/bpf/rust/instruction_introspection/src/lib.rs @@ -26,14 +26,14 @@ fn process_instruction( return Err(ProgramError::InvalidAccountData); } - let instruction = instructions::get_instruction( + let instruction = instructions::load_instruction_at( secp_instruction_index as usize, &instruction_accounts.try_borrow_data()?, ) .map_err(|_| ProgramError::InvalidAccountData)?; 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; assert_eq!(current_instruction, my_index); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index eb783ece0a..893973183d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1982,6 +1982,8 @@ impl Bank { &self.rent_collector, log_collector.clone(), executors.clone(), + self.cluster_type(), + self.epoch(), ); Self::refcells_to_accounts( diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index c6a88af7dd..4edca80ecf 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -10,6 +10,7 @@ use solana_sdk::{ ComputeBudget, ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext, Executor, InvokeContext, Logger, ProcessInstruction, ProcessInstructionWithContext, }, + genesis_config::ClusterType, instruction::{CompiledInstruction, InstructionError}, message::Message, native_loader, @@ -656,6 +657,7 @@ impl MessageProcessor { /// 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 accounts are committed back to the bank only if this function returns Ok(_). + #[allow(clippy::too_many_arguments)] fn execute_instruction( &self, message: &Message, @@ -666,17 +668,21 @@ impl MessageProcessor { log_collector: Option>, executors: Rc>, instruction_index: usize, + cluster_type: ClusterType, + epoch: Epoch, ) -> Result<(), InstructionError> { // Fixup the special instructions key if present // before the account pre-values are taken care of - for (i, key) in message.account_keys.iter().enumerate() { - if solana_sdk::sysvar::instructions::check_id(key) { - let mut mut_account_ref = accounts[i].borrow_mut(); - solana_sdk::sysvar::instructions::store_current_instruction( - &mut mut_account_ref.data, - instruction_index as u16, - ); - break; + if solana_sdk::sysvar::instructions::is_enabled(epoch, cluster_type) { + for (i, key) in message.account_keys.iter().enumerate() { + if solana_sdk::sysvar::instructions::check_id(key) { + let mut mut_account_ref = accounts[i].borrow_mut(); + solana_sdk::sysvar::instructions::store_current_index( + &mut mut_account_ref.data, + instruction_index as u16, + ); + break; + } } } @@ -716,6 +722,8 @@ impl MessageProcessor { rent_collector: &RentCollector, log_collector: Option>, executors: Rc>, + cluster_type: ClusterType, + epoch: Epoch, ) -> Result<(), TransactionError> { for (instruction_index, instruction) in message.instructions.iter().enumerate() { self.execute_instruction( @@ -727,6 +735,8 @@ impl MessageProcessor { log_collector.clone(), executors.clone(), instruction_index, + cluster_type, + epoch, ) .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; } @@ -1319,6 +1329,8 @@ mod tests { &rent_collector, None, executors.clone(), + ClusterType::Development, + 0, ); assert_eq!(result, Ok(())); assert_eq!(accounts[0].borrow().lamports, 100); @@ -1340,6 +1352,8 @@ mod tests { &rent_collector, None, executors.clone(), + ClusterType::Development, + 0, ); assert_eq!( result, @@ -1365,6 +1379,8 @@ mod tests { &rent_collector, None, executors, + ClusterType::Development, + 0, ); assert_eq!( result, @@ -1473,6 +1489,8 @@ mod tests { &rent_collector, None, executors.clone(), + ClusterType::Development, + 0, ); assert_eq!( result, @@ -1498,6 +1516,8 @@ mod tests { &rent_collector, None, executors.clone(), + ClusterType::Development, + 0, ); assert_eq!(result, Ok(())); @@ -1520,6 +1540,8 @@ mod tests { &rent_collector, None, executors, + ClusterType::Development, + 0, ); assert_eq!(result, Ok(())); assert_eq!(accounts[0].borrow().lamports, 80); diff --git a/sdk/benches/serialize_instructions.rs b/sdk/benches/serialize_instructions.rs index dc80829ed5..45bf994cd0 100644 --- a/sdk/benches/serialize_instructions.rs +++ b/sdk/benches/serialize_instructions.rs @@ -47,7 +47,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) { let serialized = message.serialize_instructions(); b.iter(|| { 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 serialized = message.serialize_instructions(); b.iter(|| { - test::black_box(instructions::get_instruction(3, &serialized).unwrap()); + test::black_box(instructions::load_instruction_at(3, &serialized).unwrap()); }); } diff --git a/sdk/src/sysvar/instructions.rs b/sdk/src/sysvar/instructions.rs index 68425d7da3..e68499f845 100644 --- a/sdk/src/sysvar/instructions.rs +++ b/sdk/src/sysvar/instructions.rs @@ -7,7 +7,7 @@ use crate::sysvar::Sysvar; pub type Instructions = Vec; -crate::declare_sysvar_id!("instructions1111111111111111111111111111111", Instructions); +crate::declare_sysvar_id!("Sysvar1nstructions1111111111111111111111111", Instructions); impl Sysvar for Instructions {} @@ -21,19 +21,19 @@ pub fn is_enabled(_epoch: Epoch, cluster_type: ClusterType) -> bool { 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 len = data.len(); instr_fixed_data.copy_from_slice(&data[len - 2..len]); 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; data[last_index..last_index + 2].copy_from_slice(&instruction_index.to_le_bytes()); } -pub fn get_instruction(index: usize, data: &[u8]) -> Result { +pub fn load_instruction_at(index: usize, data: &[u8]) -> Result { solana_sdk::message::Message::deserialize_instruction(index, data) } @@ -42,10 +42,10 @@ mod tests { use super::*; #[test] - fn test_get_store_instruction() { + fn test_load_store_instruction() { let mut data = [4u8; 10]; - store_current_instruction(&mut data, 3); - assert_eq!(get_current_instruction(&data), 3); + store_current_index(&mut data, 3); + assert_eq!(load_current_index(&data), 3); assert_eq!([4u8; 8], data[0..8]); } }