From e07dac3e0452eca7b40710745dc881001866e5b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 31 Dec 2021 13:21:58 +0100 Subject: [PATCH 1/4] Turns compute_units_consumed of ProcessInstructionResult into a &mut parameter. --- program-runtime/src/invoke_context.rs | 119 ++++++++++---------------- program-test/src/lib.rs | 3 +- programs/bpf_loader/src/syscalls.rs | 3 +- runtime/src/message_processor.rs | 9 +- 4 files changed, 51 insertions(+), 83 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 7a12c1278b0..9a1cd3908f2 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -28,12 +28,6 @@ use { pub type ProcessInstructionWithContext = fn(usize, &[u8], &mut InvokeContext) -> Result<(), InstructionError>; -#[derive(Debug, PartialEq)] -pub struct ProcessInstructionResult { - pub compute_units_consumed: u64, - pub result: Result<(), InstructionError>, -} - #[derive(Clone)] pub struct BuiltinProgram { pub program_id: Pubkey, @@ -521,13 +515,14 @@ impl<'a> InvokeContext<'a> { prev_account_sizes.push((instruction_account.index_in_transaction, account_length)); } + let mut compute_units_consumed = 0; self.process_instruction( &instruction.data, &instruction_accounts, Some(&caller_write_privileges), &program_indices, - ) - .result?; + &mut compute_units_consumed, + )?; // Verify the called program has not misbehaved let do_support_realloc = self.feature_set.is_active(&do_support_realloc::id()); @@ -716,7 +711,9 @@ impl<'a> InvokeContext<'a> { instruction_accounts: &[InstructionAccount], caller_write_privileges: Option<&[bool]>, program_indices: &[usize], - ) -> ProcessInstructionResult { + compute_units_consumed: &mut u64, + ) -> Result<(), InstructionError> { + *compute_units_consumed = 0; let program_id = program_indices .last() .map(|index| *self.transaction_context.get_key_of_account_at_index(*index)) @@ -729,13 +726,8 @@ impl<'a> InvokeContext<'a> { } } else { // Verify the calling program hasn't misbehaved - let result = self.verify_and_update(instruction_accounts, caller_write_privileges); - if result.is_err() { - return ProcessInstructionResult { - compute_units_consumed: 0, - result, - }; - } + self.verify_and_update(instruction_accounts, caller_write_privileges)?; + // Record instruction if let Some(instruction_recorder) = &self.instruction_recorder { let compiled_instruction = CompiledInstruction { @@ -755,7 +747,6 @@ impl<'a> InvokeContext<'a> { } } - let mut compute_units_consumed = 0; let result = self .push(instruction_accounts, program_indices) .and_then(|_| { @@ -763,7 +754,7 @@ impl<'a> InvokeContext<'a> { let pre_remaining_units = self.compute_meter.borrow().get_remaining(); let execution_result = self.process_executable_chain(instruction_data); let post_remaining_units = self.compute_meter.borrow().get_remaining(); - compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); + *compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); execution_result?; // Verify the called program has not misbehaved @@ -776,10 +767,7 @@ impl<'a> InvokeContext<'a> { // Pop the invoke_stack to restore previous state self.pop(); - ProcessInstructionResult { - compute_units_consumed, - result, - } + result } /// Calls the instruction's program entrypoint method @@ -1085,7 +1073,7 @@ mod tests { ModifyNotOwned, ModifyReadonly, ConsumeComputeUnits { - compute_units_consumed: u64, + compute_units_to_consume: u64, desired_result: Result<(), InstructionError>, }, } @@ -1206,13 +1194,13 @@ mod tests { .data_as_mut_slice()[0] = 1 } MockInstruction::ConsumeComputeUnits { - compute_units_consumed, + compute_units_to_consume, desired_result, } => { invoke_context .get_compute_meter() .borrow_mut() - .consume(compute_units_consumed) + .consume(compute_units_to_consume) .unwrap(); return desired_result; } @@ -1395,6 +1383,7 @@ mod tests { invoke_context .push(&instruction_accounts, &program_indices[..1]) .unwrap(); + let mut compute_units_consumed = 0; // not owned account modified by the caller (before the invoke) transaction_context @@ -1402,14 +1391,13 @@ mod tests { .borrow_mut() .data_as_mut_slice()[0] = 1; assert_eq!( - invoke_context - .process_instruction( - &instruction.data, - &instruction_accounts, - None, - &program_indices[1..], - ) - .result, + invoke_context.process_instruction( + &instruction.data, + &instruction_accounts, + None, + &program_indices[1..], + &mut compute_units_consumed, + ), Err(InstructionError::ExternalAccountDataModified) ); transaction_context @@ -1423,14 +1411,13 @@ mod tests { .borrow_mut() .data_as_mut_slice()[0] = 1; assert_eq!( - invoke_context - .process_instruction( - &instruction.data, - &instruction_accounts, - None, - &program_indices[1..], - ) - .result, + invoke_context.process_instruction( + &instruction.data, + &instruction_accounts, + None, + &program_indices[1..], + &mut compute_units_consumed, + ), Err(InstructionError::ReadonlyDataModified) ); transaction_context @@ -1441,33 +1428,15 @@ mod tests { invoke_context.pop(); let cases = vec![ - ( - MockInstruction::NoopSuccess, - ProcessInstructionResult { - result: Ok(()), - compute_units_consumed: 0, - }, - ), + (MockInstruction::NoopSuccess, Ok(())), ( MockInstruction::NoopFail, - ProcessInstructionResult { - result: Err(InstructionError::GenericError), - compute_units_consumed: 0, - }, - ), - ( - MockInstruction::ModifyOwned, - ProcessInstructionResult { - result: Ok(()), - compute_units_consumed: 0, - }, + Err(InstructionError::GenericError), ), + (MockInstruction::ModifyOwned, Ok(())), ( MockInstruction::ModifyNotOwned, - ProcessInstructionResult { - result: Err(InstructionError::ExternalAccountDataModified), - compute_units_consumed: 0, - }, + Err(InstructionError::ExternalAccountDataModified), ), ]; for case in cases { @@ -1482,6 +1451,7 @@ mod tests { &instruction_accounts, None, &program_indices[1..], + &mut compute_units_consumed, ), case.1 ); @@ -1683,15 +1653,15 @@ mod tests { let transaction_context = TransactionContext::new(accounts, 1); let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); - let compute_units_consumed = 10; - let desired_results = vec![Ok(()), Err(InstructionError::GenericError)]; + let compute_units_to_consume = 10; + let expected_results = vec![Ok(()), Err(InstructionError::GenericError)]; - for desired_result in desired_results { + for expected_result in expected_results { let instruction = Instruction::new_with_bincode( callee_program_id, &MockInstruction::ConsumeComputeUnits { - compute_units_consumed, - desired_result: desired_result.clone(), + compute_units_to_consume, + desired_result: expected_result.clone(), }, metas.clone(), ); @@ -1699,24 +1669,21 @@ mod tests { .push(&instruction_accounts, &program_indices[..1]) .unwrap(); + let mut compute_units_consumed = 0; let result = invoke_context.process_instruction( &instruction.data, &instruction_accounts, None, &program_indices[1..], + &mut compute_units_consumed, ); // Because the instruction had compute cost > 0, then regardless of the execution result, // the number of compute units consumed should be a non-default which is something greater // than zero. - assert!(result.compute_units_consumed > 0); - assert_eq!( - result, - ProcessInstructionResult { - compute_units_consumed, - result: desired_result, - } - ); + assert!(compute_units_consumed > 0); + assert_eq!(compute_units_consumed, compute_units_to_consume); + assert_eq!(result, expected_result); } } } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 1a193c5d7c3..23005e029ed 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -276,14 +276,15 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { } } + let mut compute_units_consumed = 0; invoke_context .process_instruction( &instruction.data, &instruction_accounts, Some(&caller_write_privileges), &program_indices, + &mut compute_units_consumed, ) - .result .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?; // Copy invoke_context accounts modifications into caller's account_info diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 8f6e6fe78d6..e04640f5701 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -2384,14 +2384,15 @@ fn call<'a, 'b: 'a>( )?; // Process instruction + let mut compute_units_consumed = 0; invoke_context .process_instruction( &instruction.data, &instruction_accounts, Some(&caller_write_privileges), &program_indices, + &mut compute_units_consumed, ) - .result .map_err(SyscallError::InstructionError)?; // Copy results back to caller diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 7bbd9faeb80..96ccebc215c 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -3,7 +3,7 @@ use { solana_measure::measure::Measure, solana_program_runtime::{ instruction_recorder::InstructionRecorder, - invoke_context::{BuiltinProgram, Executors, InvokeContext, ProcessInstructionResult}, + invoke_context::{BuiltinProgram, Executors, InvokeContext}, log_collector::LogCollector, timings::ExecuteDetailsTimings, }, @@ -128,14 +128,13 @@ impl MessageProcessor { }) .collect::>(); let mut time = Measure::start("execute_instruction"); - let ProcessInstructionResult { - compute_units_consumed, - result, - } = invoke_context.process_instruction( + let mut compute_units_consumed = 0; + let result = invoke_context.process_instruction( &instruction.data, &instruction_accounts, None, program_indices, + &mut compute_units_consumed, ); time.stop(); timings.accumulate_program( From a05950df40ab4c10f3b4d804460809fcc1a507ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 31 Dec 2021 13:25:48 +0100 Subject: [PATCH 2/4] Removes second nesting level from test_process_instruction_compute_budget(). --- program-runtime/src/invoke_context.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 9a1cd3908f2..88016b718e6 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -1612,7 +1612,6 @@ mod tests { #[test] fn test_process_instruction_compute_budget() { - let caller_program_id = solana_sdk::pubkey::new_rand(); let callee_program_id = solana_sdk::pubkey::new_rand(); let builtin_programs = &[BuiltinProgram { program_id: callee_program_id, @@ -1622,7 +1621,6 @@ mod tests { let owned_account = AccountSharedData::new(42, 1, &callee_program_id); let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand()); - let loader_account = AccountSharedData::new(0, 0, &native_loader::id()); let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); program_account.set_executable(true); @@ -1630,10 +1628,9 @@ mod tests { (solana_sdk::pubkey::new_rand(), owned_account), (solana_sdk::pubkey::new_rand(), not_owned_account), (solana_sdk::pubkey::new_rand(), readonly_account), - (caller_program_id, loader_account), (callee_program_id, program_account), ]; - let program_indices = [3, 4]; + let program_indices = [3]; let metas = vec![ AccountMeta::new(accounts[0].0, false), @@ -1665,16 +1662,13 @@ mod tests { }, metas.clone(), ); - invoke_context - .push(&instruction_accounts, &program_indices[..1]) - .unwrap(); let mut compute_units_consumed = 0; let result = invoke_context.process_instruction( &instruction.data, &instruction_accounts, None, - &program_indices[1..], + &program_indices, &mut compute_units_consumed, ); From 5bcb2ff88b7d3528610f04e27c73efe29bfe3d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 31 Dec 2021 13:59:13 +0100 Subject: [PATCH 3/4] Makes test_process_cross_program and test_native_invoke symmetric. --- program-runtime/src/invoke_context.rs | 146 +++++++++++++------------- 1 file changed, 75 insertions(+), 71 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 88016b718e6..b3c15751079 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -1335,7 +1335,6 @@ mod tests { #[test] fn test_process_cross_program() { - let caller_program_id = solana_sdk::pubkey::new_rand(); let callee_program_id = solana_sdk::pubkey::new_rand(); let builtin_programs = &[BuiltinProgram { program_id: callee_program_id, @@ -1348,85 +1347,87 @@ mod tests { let loader_account = AccountSharedData::new(0, 0, &native_loader::id()); let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); program_account.set_executable(true); - let accounts = vec![ (solana_sdk::pubkey::new_rand(), owned_account), (solana_sdk::pubkey::new_rand(), not_owned_account), (solana_sdk::pubkey::new_rand(), readonly_account), - (caller_program_id, loader_account), (callee_program_id, program_account), + (solana_sdk::pubkey::new_rand(), loader_account), ]; - let program_indices = [3, 4]; - let metas = vec![ AccountMeta::new(accounts[0].0, false), AccountMeta::new(accounts[1].0, false), AccountMeta::new_readonly(accounts[2].0, false), ]; - let instruction_accounts = metas - .iter() - .enumerate() - .map(|(index_in_transaction, account_meta)| InstructionAccount { + let instruction_accounts = (0..4) + .map(|index_in_transaction| InstructionAccount { index_in_transaction, - index_in_caller: program_indices.len() + index_in_transaction, - is_signer: account_meta.is_signer, - is_writable: account_meta.is_writable, + index_in_caller: 1 + index_in_transaction, + is_signer: false, + is_writable: index_in_transaction < 2, }) .collect::>(); - let instruction = Instruction::new_with_bincode( + let transaction_context = TransactionContext::new(accounts, 2); + let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); + invoke_context.push(&instruction_accounts, &[4]).unwrap(); + + let inner_instruction = Instruction::new_with_bincode( callee_program_id, &MockInstruction::NoopSuccess, metas.clone(), ); - let transaction_context = TransactionContext::new(accounts, 1); - let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); - invoke_context - .push(&instruction_accounts, &program_indices[..1]) + let (inner_instruction_accounts, caller_write_privileges, program_indices) = invoke_context + .prepare_instruction(&inner_instruction, &[]) .unwrap(); let mut compute_units_consumed = 0; // not owned account modified by the caller (before the invoke) - transaction_context + invoke_context + .transaction_context .get_account_at_index(1) .borrow_mut() .data_as_mut_slice()[0] = 1; assert_eq!( invoke_context.process_instruction( - &instruction.data, - &instruction_accounts, - None, - &program_indices[1..], + &inner_instruction.data, + &inner_instruction_accounts, + Some(&caller_write_privileges), + &program_indices, &mut compute_units_consumed, ), Err(InstructionError::ExternalAccountDataModified) ); - transaction_context + invoke_context + .transaction_context .get_account_at_index(1) .borrow_mut() .data_as_mut_slice()[0] = 0; // readonly account modified by the invoker - transaction_context + invoke_context + .transaction_context .get_account_at_index(2) .borrow_mut() .data_as_mut_slice()[0] = 1; assert_eq!( invoke_context.process_instruction( - &instruction.data, - &instruction_accounts, - None, - &program_indices[1..], + &inner_instruction.data, + &inner_instruction_accounts, + Some(&caller_write_privileges), + &program_indices, &mut compute_units_consumed, ), Err(InstructionError::ReadonlyDataModified) ); - transaction_context + invoke_context + .transaction_context .get_account_at_index(2) .borrow_mut() .data_as_mut_slice()[0] = 0; invoke_context.pop(); + // Other test cases let cases = vec![ (MockInstruction::NoopSuccess, Ok(())), ( @@ -1438,21 +1439,30 @@ mod tests { MockInstruction::ModifyNotOwned, Err(InstructionError::ExternalAccountDataModified), ), + (MockInstruction::ModifyOwned, Ok(())), + ( + MockInstruction::ModifyReadonly, + Err(InstructionError::ReadonlyDataModified), + ), ]; for case in cases { - let instruction = + invoke_context.push(&instruction_accounts, &[4]).unwrap(); + let inner_instruction = Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone()); - invoke_context - .push(&instruction_accounts, &program_indices[..1]) - .unwrap(); + let (inner_instruction_accounts, caller_write_privileges, program_indices) = + invoke_context + .prepare_instruction(&inner_instruction, &[]) + .unwrap(); assert_eq!( - invoke_context.process_instruction( - &instruction.data, - &instruction_accounts, - None, - &program_indices[1..], - &mut compute_units_consumed, - ), + invoke_context + .process_instruction( + &inner_instruction.data, + &inner_instruction_accounts, + Some(&caller_write_privileges), + &program_indices, + &mut compute_units_consumed, + ) + .map(|_| ()), case.1 ); invoke_context.pop(); @@ -1470,6 +1480,7 @@ mod tests { let owned_account = AccountSharedData::new(42, 1, &callee_program_id); let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand()); + let loader_account = AccountSharedData::new(0, 0, &native_loader::id()); let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); program_account.set_executable(true); let accounts = vec![ @@ -1477,61 +1488,59 @@ mod tests { (solana_sdk::pubkey::new_rand(), not_owned_account), (solana_sdk::pubkey::new_rand(), readonly_account), (callee_program_id, program_account), + (solana_sdk::pubkey::new_rand(), loader_account), ]; - let program_indices = [3]; - let metas = vec![ AccountMeta::new(accounts[0].0, false), AccountMeta::new(accounts[1].0, false), AccountMeta::new_readonly(accounts[2].0, false), - AccountMeta::new_readonly(accounts[3].0, false), ]; - let instruction_accounts = metas - .iter() - .enumerate() - .map(|(index_in_transaction, account_meta)| InstructionAccount { + let instruction_accounts = (0..4) + .map(|index_in_transaction| InstructionAccount { index_in_transaction, - index_in_caller: program_indices.len() + index_in_transaction, - is_signer: account_meta.is_signer, - is_writable: account_meta.is_writable, + index_in_caller: 1 + index_in_transaction, + is_signer: false, + is_writable: index_in_transaction < 2, }) .collect::>(); - let callee_instruction = Instruction::new_with_bincode( + let transaction_context = TransactionContext::new(accounts, 2); + let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); + invoke_context.push(&instruction_accounts, &[4]).unwrap(); + + let inner_instruction = Instruction::new_with_bincode( callee_program_id, &MockInstruction::NoopSuccess, metas.clone(), ); - let transaction_context = TransactionContext::new(accounts, 1); - let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); - invoke_context - .push(&instruction_accounts, &program_indices) - .unwrap(); - // not owned account modified by the invoker - transaction_context + invoke_context + .transaction_context .get_account_at_index(1) .borrow_mut() .data_as_mut_slice()[0] = 1; assert_eq!( - invoke_context.native_invoke(callee_instruction.clone(), &[]), + invoke_context.native_invoke(inner_instruction.clone(), &[]), Err(InstructionError::ExternalAccountDataModified) ); - transaction_context + invoke_context + .transaction_context .get_account_at_index(1) .borrow_mut() .data_as_mut_slice()[0] = 0; // readonly account modified by the invoker - transaction_context + invoke_context + .transaction_context .get_account_at_index(2) .borrow_mut() .data_as_mut_slice()[0] = 1; assert_eq!( - invoke_context.native_invoke(callee_instruction, &[]), + invoke_context.native_invoke(inner_instruction, &[]), Err(InstructionError::ReadonlyDataModified) ); - transaction_context + invoke_context + .transaction_context .get_account_at_index(2) .borrow_mut() .data_as_mut_slice()[0] = 0; @@ -1556,15 +1565,10 @@ mod tests { ), ]; for case in cases { - let callee_instruction = + invoke_context.push(&instruction_accounts, &[4]).unwrap(); + let inner_instruction = Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone()); - invoke_context - .push(&instruction_accounts, &program_indices) - .unwrap(); - assert_eq!( - invoke_context.native_invoke(callee_instruction, &[]), - case.1 - ); + assert_eq!(invoke_context.native_invoke(inner_instruction, &[]), case.1); invoke_context.pop(); } } From 153cbdeb14c1de1d2eac52f266cb0423881e2a5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 31 Dec 2021 14:11:36 +0100 Subject: [PATCH 4/4] Unifies test_process_cross_program(), test_native_invoke() and test_process_instruction_compute_budget() into test_process_instruction(). --- program-runtime/src/invoke_context.rs | 313 ++++++-------------------- 1 file changed, 73 insertions(+), 240 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index b3c15751079..75e10294eba 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -1334,7 +1334,7 @@ mod tests { } #[test] - fn test_process_cross_program() { + fn test_process_instruction() { let callee_program_id = solana_sdk::pubkey::new_rand(); let builtin_programs = &[BuiltinProgram { program_id: callee_program_id, @@ -1369,65 +1369,52 @@ mod tests { .collect::>(); let transaction_context = TransactionContext::new(accounts, 2); let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); - invoke_context.push(&instruction_accounts, &[4]).unwrap(); - let inner_instruction = Instruction::new_with_bincode( - callee_program_id, - &MockInstruction::NoopSuccess, - metas.clone(), - ); - let (inner_instruction_accounts, caller_write_privileges, program_indices) = invoke_context - .prepare_instruction(&inner_instruction, &[]) - .unwrap(); - let mut compute_units_consumed = 0; + // External modification tests + { + invoke_context.push(&instruction_accounts, &[4]).unwrap(); + let inner_instruction = Instruction::new_with_bincode( + callee_program_id, + &MockInstruction::NoopSuccess, + metas.clone(), + ); - // not owned account modified by the caller (before the invoke) - invoke_context - .transaction_context - .get_account_at_index(1) - .borrow_mut() - .data_as_mut_slice()[0] = 1; - assert_eq!( - invoke_context.process_instruction( - &inner_instruction.data, - &inner_instruction_accounts, - Some(&caller_write_privileges), - &program_indices, - &mut compute_units_consumed, - ), - Err(InstructionError::ExternalAccountDataModified) - ); - invoke_context - .transaction_context - .get_account_at_index(1) - .borrow_mut() - .data_as_mut_slice()[0] = 0; + // not owned account + invoke_context + .transaction_context + .get_account_at_index(1) + .borrow_mut() + .data_as_mut_slice()[0] = 1; + assert_eq!( + invoke_context.native_invoke(inner_instruction.clone(), &[]), + Err(InstructionError::ExternalAccountDataModified) + ); + invoke_context + .transaction_context + .get_account_at_index(1) + .borrow_mut() + .data_as_mut_slice()[0] = 0; - // readonly account modified by the invoker - invoke_context - .transaction_context - .get_account_at_index(2) - .borrow_mut() - .data_as_mut_slice()[0] = 1; - assert_eq!( - invoke_context.process_instruction( - &inner_instruction.data, - &inner_instruction_accounts, - Some(&caller_write_privileges), - &program_indices, - &mut compute_units_consumed, - ), - Err(InstructionError::ReadonlyDataModified) - ); - invoke_context - .transaction_context - .get_account_at_index(2) - .borrow_mut() - .data_as_mut_slice()[0] = 0; + // readonly account + invoke_context + .transaction_context + .get_account_at_index(2) + .borrow_mut() + .data_as_mut_slice()[0] = 1; + assert_eq!( + invoke_context.native_invoke(inner_instruction, &[]), + Err(InstructionError::ReadonlyDataModified) + ); + invoke_context + .transaction_context + .get_account_at_index(2) + .borrow_mut() + .data_as_mut_slice()[0] = 0; - invoke_context.pop(); + invoke_context.pop(); + } - // Other test cases + // Internal modification tests let cases = vec![ (MockInstruction::NoopSuccess, Ok(())), ( @@ -1439,7 +1426,6 @@ mod tests { MockInstruction::ModifyNotOwned, Err(InstructionError::ExternalAccountDataModified), ), - (MockInstruction::ModifyOwned, Ok(())), ( MockInstruction::ModifyReadonly, Err(InstructionError::ReadonlyDataModified), @@ -1449,126 +1435,44 @@ mod tests { invoke_context.push(&instruction_accounts, &[4]).unwrap(); let inner_instruction = Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone()); + assert_eq!(invoke_context.native_invoke(inner_instruction, &[]), case.1); + invoke_context.pop(); + } + + // Compute unit consumption tests + let compute_units_to_consume = 10; + let expected_results = vec![Ok(()), Err(InstructionError::GenericError)]; + for expected_result in expected_results { + invoke_context.push(&instruction_accounts, &[4]).unwrap(); + let inner_instruction = Instruction::new_with_bincode( + callee_program_id, + &MockInstruction::ConsumeComputeUnits { + compute_units_to_consume, + desired_result: expected_result.clone(), + }, + metas.clone(), + ); let (inner_instruction_accounts, caller_write_privileges, program_indices) = invoke_context .prepare_instruction(&inner_instruction, &[]) .unwrap(); - assert_eq!( - invoke_context - .process_instruction( - &inner_instruction.data, - &inner_instruction_accounts, - Some(&caller_write_privileges), - &program_indices, - &mut compute_units_consumed, - ) - .map(|_| ()), - case.1 - ); - invoke_context.pop(); - } - } - - #[test] - fn test_native_invoke() { - let callee_program_id = solana_sdk::pubkey::new_rand(); - let builtin_programs = &[BuiltinProgram { - program_id: callee_program_id, - process_instruction: mock_process_instruction, - }]; - let owned_account = AccountSharedData::new(42, 1, &callee_program_id); - let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); - let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand()); - let loader_account = AccountSharedData::new(0, 0, &native_loader::id()); - let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); - program_account.set_executable(true); - let accounts = vec![ - (solana_sdk::pubkey::new_rand(), owned_account), - (solana_sdk::pubkey::new_rand(), not_owned_account), - (solana_sdk::pubkey::new_rand(), readonly_account), - (callee_program_id, program_account), - (solana_sdk::pubkey::new_rand(), loader_account), - ]; - let metas = vec![ - AccountMeta::new(accounts[0].0, false), - AccountMeta::new(accounts[1].0, false), - AccountMeta::new_readonly(accounts[2].0, false), - ]; - let instruction_accounts = (0..4) - .map(|index_in_transaction| InstructionAccount { - index_in_transaction, - index_in_caller: 1 + index_in_transaction, - is_signer: false, - is_writable: index_in_transaction < 2, - }) - .collect::>(); - let transaction_context = TransactionContext::new(accounts, 2); - let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); - invoke_context.push(&instruction_accounts, &[4]).unwrap(); - - let inner_instruction = Instruction::new_with_bincode( - callee_program_id, - &MockInstruction::NoopSuccess, - metas.clone(), - ); - - // not owned account modified by the invoker - invoke_context - .transaction_context - .get_account_at_index(1) - .borrow_mut() - .data_as_mut_slice()[0] = 1; - assert_eq!( - invoke_context.native_invoke(inner_instruction.clone(), &[]), - Err(InstructionError::ExternalAccountDataModified) - ); - invoke_context - .transaction_context - .get_account_at_index(1) - .borrow_mut() - .data_as_mut_slice()[0] = 0; - - // readonly account modified by the invoker - invoke_context - .transaction_context - .get_account_at_index(2) - .borrow_mut() - .data_as_mut_slice()[0] = 1; - assert_eq!( - invoke_context.native_invoke(inner_instruction, &[]), - Err(InstructionError::ReadonlyDataModified) - ); - invoke_context - .transaction_context - .get_account_at_index(2) - .borrow_mut() - .data_as_mut_slice()[0] = 0; + let mut compute_units_consumed = 0; + let result = invoke_context.process_instruction( + &inner_instruction.data, + &inner_instruction_accounts, + Some(&caller_write_privileges), + &program_indices, + &mut compute_units_consumed, + ); - invoke_context.pop(); + // Because the instruction had compute cost > 0, then regardless of the execution result, + // the number of compute units consumed should be a non-default which is something greater + // than zero. + assert!(compute_units_consumed > 0); + assert_eq!(compute_units_consumed, compute_units_to_consume); + assert_eq!(result, expected_result); - // Other test cases - let cases = vec![ - (MockInstruction::NoopSuccess, Ok(())), - ( - MockInstruction::NoopFail, - Err(InstructionError::GenericError), - ), - (MockInstruction::ModifyOwned, Ok(())), - ( - MockInstruction::ModifyNotOwned, - Err(InstructionError::ExternalAccountDataModified), - ), - ( - MockInstruction::ModifyReadonly, - Err(InstructionError::ReadonlyDataModified), - ), - ]; - for case in cases { - invoke_context.push(&instruction_accounts, &[4]).unwrap(); - let inner_instruction = - Instruction::new_with_bincode(callee_program_id, &case.0, metas.clone()); - assert_eq!(invoke_context.native_invoke(inner_instruction, &[]), case.1); invoke_context.pop(); } } @@ -1613,75 +1517,4 @@ mod tests { ); invoke_context.pop(); } - - #[test] - fn test_process_instruction_compute_budget() { - let callee_program_id = solana_sdk::pubkey::new_rand(); - let builtin_programs = &[BuiltinProgram { - program_id: callee_program_id, - process_instruction: mock_process_instruction, - }]; - - let owned_account = AccountSharedData::new(42, 1, &callee_program_id); - let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand()); - let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand()); - let mut program_account = AccountSharedData::new(1, 0, &native_loader::id()); - program_account.set_executable(true); - - let accounts = vec![ - (solana_sdk::pubkey::new_rand(), owned_account), - (solana_sdk::pubkey::new_rand(), not_owned_account), - (solana_sdk::pubkey::new_rand(), readonly_account), - (callee_program_id, program_account), - ]; - let program_indices = [3]; - - let metas = vec![ - AccountMeta::new(accounts[0].0, false), - AccountMeta::new(accounts[1].0, false), - AccountMeta::new_readonly(accounts[2].0, false), - ]; - let instruction_accounts = metas - .iter() - .enumerate() - .map(|(account_index, account_meta)| InstructionAccount { - index_in_transaction: account_index, - index_in_caller: 1 + account_index, - is_signer: account_meta.is_signer, - is_writable: account_meta.is_writable, - }) - .collect::>(); - - let transaction_context = TransactionContext::new(accounts, 1); - let mut invoke_context = InvokeContext::new_mock(&transaction_context, builtin_programs); - let compute_units_to_consume = 10; - let expected_results = vec![Ok(()), Err(InstructionError::GenericError)]; - - for expected_result in expected_results { - let instruction = Instruction::new_with_bincode( - callee_program_id, - &MockInstruction::ConsumeComputeUnits { - compute_units_to_consume, - desired_result: expected_result.clone(), - }, - metas.clone(), - ); - - let mut compute_units_consumed = 0; - let result = invoke_context.process_instruction( - &instruction.data, - &instruction_accounts, - None, - &program_indices, - &mut compute_units_consumed, - ); - - // Because the instruction had compute cost > 0, then regardless of the execution result, - // the number of compute units consumed should be a non-default which is something greater - // than zero. - assert!(compute_units_consumed > 0); - assert_eq!(compute_units_consumed, compute_units_to_consume); - assert_eq!(result, expected_result); - } - } }