diff --git a/cli/src/program.rs b/cli/src/program.rs index b2c2135d2..1b3431968 100644 --- a/cli/src/program.rs +++ b/cli/src/program.rs @@ -46,6 +46,7 @@ use { signature::{keypair_from_seed, read_keypair_file, Keypair, Signature, Signer}, system_instruction::{self, SystemError}, system_program, + sysvar::rent::Rent, transaction::{Transaction, TransactionError}, transaction_context::TransactionContext, }, @@ -2060,7 +2061,7 @@ fn read_and_verify_elf(program_location: &str) -> Result, Box InvokeContext<'a> { { self.current_compute_budget = self.compute_budget; - self.pre_accounts = Vec::with_capacity(instruction_accounts.len()); - - for (instruction_account_index, instruction_account) in - instruction_accounts.iter().enumerate() + if !self + .feature_set + .is_active(&enable_early_verification_of_account_modifications::id()) { - if instruction_account_index != instruction_account.index_in_callee { - continue; // Skip duplicate account - } - if instruction_account.index_in_transaction - >= self.transaction_context.get_number_of_accounts() + self.pre_accounts = Vec::with_capacity(instruction_accounts.len()); + for (instruction_account_index, instruction_account) in + instruction_accounts.iter().enumerate() { - return Err(InstructionError::MissingAccount); + if instruction_account_index != instruction_account.index_in_callee { + continue; // Skip duplicate account + } + if instruction_account.index_in_transaction + >= self.transaction_context.get_number_of_accounts() + { + return Err(InstructionError::MissingAccount); + } + let account = self + .transaction_context + .get_account_at_index(instruction_account.index_in_transaction)? + .borrow() + .clone(); + self.pre_accounts.push(PreAccount::new( + self.transaction_context.get_key_of_account_at_index( + instruction_account.index_in_transaction, + )?, + account, + )); } - let account = self - .transaction_context - .get_account_at_index(instruction_account.index_in_transaction)? - .borrow() - .clone(); - self.pre_accounts.push(PreAccount::new( - self.transaction_context - .get_key_of_account_at_index(instruction_account.index_in_transaction)?, - account, - )); } } else { let contains = (0..self @@ -831,26 +837,35 @@ impl<'a> InvokeContext<'a> { .get_instruction_context_stack_height(); let is_top_level_instruction = nesting_level == 0; if !is_top_level_instruction { - // Verify the calling program hasn't misbehaved - let mut verify_caller_time = Measure::start("verify_caller_time"); - let verify_caller_result = self.verify_and_update(instruction_accounts, true); - verify_caller_time.stop(); - saturating_add_assign!( - timings - .execute_accessories - .process_instructions - .verify_caller_us, - verify_caller_time.as_us() - ); - verify_caller_result?; + if !self + .feature_set + .is_active(&enable_early_verification_of_account_modifications::id()) + { + // Verify the calling program hasn't misbehaved + let mut verify_caller_time = Measure::start("verify_caller_time"); + let verify_caller_result = self.verify_and_update(instruction_accounts, true); + verify_caller_time.stop(); + saturating_add_assign!( + timings + .execute_accessories + .process_instructions + .verify_caller_us, + verify_caller_time.as_us() + ); + verify_caller_result?; + } if !self .feature_set .is_active(&record_instruction_in_transaction_context_push::id()) { + let instruction_accounts_lamport_sum = self + .transaction_context + .instruction_accounts_lamport_sum(instruction_accounts)?; self.transaction_context .record_instruction(InstructionContext::new( nesting_level, + instruction_accounts_lamport_sum, program_indices, instruction_accounts, instruction_data, @@ -861,22 +876,29 @@ impl<'a> InvokeContext<'a> { self.push(instruction_accounts, program_indices, instruction_data)?; self.process_executable_chain(compute_units_consumed, timings) .and_then(|_| { - // Verify the called program has not misbehaved - let mut verify_callee_time = Measure::start("verify_callee_time"); - let result = if is_top_level_instruction { - self.verify(instruction_accounts, program_indices) + if self + .feature_set + .is_active(&enable_early_verification_of_account_modifications::id()) + { + Ok(()) } else { - self.verify_and_update(instruction_accounts, false) - }; - verify_callee_time.stop(); - saturating_add_assign!( - timings - .execute_accessories - .process_instructions - .verify_callee_us, - verify_callee_time.as_us() - ); - result + // Verify the called program has not misbehaved + let mut verify_callee_time = Measure::start("verify_callee_time"); + let result = if is_top_level_instruction { + self.verify(instruction_accounts, program_indices) + } else { + self.verify_and_update(instruction_accounts, false) + }; + verify_callee_time.stop(); + saturating_add_assign!( + timings + .execute_accessories + .process_instructions + .verify_callee_us, + verify_callee_time.as_us() + ); + result + } }) // MUST pop if and only if `push` succeeded, independent of `result`. // Thus, the `.and()` instead of an `.and_then()`. @@ -1138,6 +1160,7 @@ pub fn with_mock_invoke_context R>( prepare_mock_invoke_context(transaction_accounts, instruction_accounts, &program_indices); let mut transaction_context = TransactionContext::new( preparation.transaction_accounts, + Some(Rent::default()), ComputeBudget::default().max_invoke_depth.saturating_add(1), 1, ); @@ -1168,6 +1191,7 @@ pub fn mock_process_instruction( .push((*loader_id, processor_account)); let mut transaction_context = TransactionContext::new( preparation.transaction_accounts, + Some(Rent::default()), ComputeBudget::default().max_invoke_depth.saturating_add(1), 1, ); @@ -1184,10 +1208,9 @@ pub fn mock_process_instruction( &program_indices, instruction_data, ) - .and_then(|_| process_instruction(1, &mut invoke_context)) - .and_then(|_| invoke_context.verify(&preparation.instruction_accounts, &program_indices)); - invoke_context.pop().unwrap(); - assert_eq!(result, expected_result); + .and_then(|_| process_instruction(1, &mut invoke_context)); + let pop_result = invoke_context.pop(); + assert_eq!(result.and(pop_result), expected_result); let mut transaction_accounts = transaction_context.deconstruct_without_keys().unwrap(); transaction_accounts.pop(); transaction_accounts @@ -1199,7 +1222,7 @@ mod tests { super::*, crate::compute_budget, serde::{Deserialize, Serialize}, - solana_sdk::account::{ReadableAccount, WritableAccount}, + solana_sdk::account::WritableAccount, }; #[derive(Debug, Serialize, Deserialize)] @@ -1209,6 +1232,8 @@ mod tests { ModifyOwned, ModifyNotOwned, ModifyReadonly, + UnbalancedPush, + UnbalancedPop, ConsumeComputeUnits { compute_units_to_consume: u64, desired_result: Result<(), InstructionError>, @@ -1256,6 +1281,15 @@ mod tests { let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); let program_id = instruction_context.get_last_program_key(transaction_context)?; + let instruction_accounts = (0..4) + .map(|instruction_account_index| InstructionAccount { + index_in_transaction: instruction_account_index, + index_in_caller: instruction_account_index, + index_in_callee: instruction_account_index, + is_signer: false, + is_writable: false, + }) + .collect::>(); assert_eq!( program_id, instruction_context @@ -1277,16 +1311,43 @@ mod tests { MockInstruction::NoopFail => return Err(InstructionError::GenericError), MockInstruction::ModifyOwned => instruction_context .try_borrow_instruction_account(transaction_context, 0)? - .set_data(&[1]) - .unwrap(), + .set_data(&[1])?, MockInstruction::ModifyNotOwned => instruction_context .try_borrow_instruction_account(transaction_context, 1)? - .set_data(&[1]) - .unwrap(), + .set_data(&[1])?, MockInstruction::ModifyReadonly => instruction_context .try_borrow_instruction_account(transaction_context, 2)? - .set_data(&[1]) - .unwrap(), + .set_data(&[1])?, + MockInstruction::UnbalancedPush => { + instruction_context + .try_borrow_instruction_account(transaction_context, 0)? + .checked_add_lamports(1)?; + let program_id = *transaction_context.get_key_of_account_at_index(3)?; + let metas = vec![ + AccountMeta::new_readonly( + *transaction_context.get_key_of_account_at_index(0)?, + false, + ), + AccountMeta::new_readonly( + *transaction_context.get_key_of_account_at_index(1)?, + false, + ), + ]; + let inner_instruction = Instruction::new_with_bincode( + program_id, + &MockInstruction::NoopSuccess, + metas, + ); + let result = invoke_context.push(&instruction_accounts, &[3], &[]); + assert_eq!(result, Err(InstructionError::UnbalancedInstruction)); + result?; + invoke_context + .native_invoke(inner_instruction, &[]) + .and(invoke_context.pop())?; + } + MockInstruction::UnbalancedPop => instruction_context + .try_borrow_instruction_account(transaction_context, 0)? + .checked_add_lamports(1)?, MockInstruction::ConsumeComputeUnits { compute_units_to_consume, desired_result, @@ -1294,14 +1355,12 @@ mod tests { invoke_context .get_compute_meter() .borrow_mut() - .consume(compute_units_to_consume) - .unwrap(); + .consume(compute_units_to_consume)?; return desired_result; } MockInstruction::Resize { new_len } => instruction_context .try_borrow_instruction_account(transaction_context, 0)? - .set_data(&vec![0; new_len as usize]) - .unwrap(), + .set_data(&vec![0; new_len as usize])?, } } else { return Err(InstructionError::InvalidInstructionData); @@ -1310,7 +1369,7 @@ mod tests { } #[test] - fn test_invoke_context() { + fn test_instruction_stack_height() { const MAX_DEPTH: usize = 10; let mut invoke_stack = vec![]; let mut accounts = vec![]; @@ -1342,8 +1401,12 @@ mod tests { is_writable: false, }); } - let mut transaction_context = - TransactionContext::new(accounts, ComputeBudget::default().max_invoke_depth, 1); + let mut transaction_context = TransactionContext::new( + accounts, + Some(Rent::default()), + ComputeBudget::default().max_invoke_depth, + 1, + ); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); // Check call depth increases and has a limit @@ -1358,107 +1421,6 @@ mod tests { } assert_ne!(depth_reached, 0); assert!(depth_reached < MAX_DEPTH); - - // Mock each invocation - for owned_index in (1..depth_reached).rev() { - let not_owned_index = owned_index - 1; - let instruction_accounts = vec![ - InstructionAccount { - index_in_transaction: not_owned_index, - index_in_caller: not_owned_index, - index_in_callee: 0, - is_signer: false, - is_writable: true, - }, - InstructionAccount { - index_in_transaction: owned_index, - index_in_caller: owned_index, - index_in_callee: 1, - is_signer: false, - is_writable: true, - }, - ]; - - // modify account owned by the program - *invoke_context - .transaction_context - .get_account_at_index(owned_index) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = (MAX_DEPTH + owned_index) as u8; - invoke_context - .verify_and_update(&instruction_accounts, false) - .unwrap(); - assert_eq!( - *invoke_context - .pre_accounts - .get(owned_index) - .unwrap() - .data() - .first() - .unwrap(), - (MAX_DEPTH + owned_index) as u8 - ); - - // modify account not owned by the program - let data = *invoke_context - .transaction_context - .get_account_at_index(not_owned_index) - .unwrap() - .borrow_mut() - .data() - .first() - .unwrap(); - *invoke_context - .transaction_context - .get_account_at_index(not_owned_index) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = (MAX_DEPTH + not_owned_index) as u8; - assert_eq!( - invoke_context.verify_and_update(&instruction_accounts, false), - Err(InstructionError::ExternalAccountDataModified) - ); - assert_eq!( - *invoke_context - .pre_accounts - .get(not_owned_index) - .unwrap() - .data() - .first() - .unwrap(), - data - ); - *invoke_context - .transaction_context - .get_account_at_index(not_owned_index) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = data; - - invoke_context.pop().unwrap(); - } - } - - #[test] - fn test_invoke_context_verify() { - let accounts = vec![(solana_sdk::pubkey::new_rand(), AccountSharedData::default())]; - let instruction_accounts = vec![]; - let program_indices = vec![0]; - let mut transaction_context = TransactionContext::new(accounts, 1, 1); - let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); - invoke_context - .push(&instruction_accounts, &program_indices, &[]) - .unwrap(); - assert!(invoke_context - .verify(&instruction_accounts, &program_indices) - .is_ok()); } #[test] @@ -1496,69 +1458,12 @@ mod tests { is_writable: instruction_account_index < 2, }) .collect::>(); - let mut transaction_context = TransactionContext::new(accounts, 2, 8); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 2, 9); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, builtin_programs); - // 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 - *invoke_context - .transaction_context - .get_account_at_index(1) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = 1; - assert_eq!( - invoke_context.native_invoke(inner_instruction.clone(), &[]), - Err(InstructionError::ExternalAccountDataModified) - ); - *invoke_context - .transaction_context - .get_account_at_index(1) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = 0; - - // readonly account - *invoke_context - .transaction_context - .get_account_at_index(2) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = 1; - assert_eq!( - invoke_context.native_invoke(inner_instruction, &[]), - Err(InstructionError::ReadonlyDataModified) - ); - *invoke_context - .transaction_context - .get_account_at_index(2) - .unwrap() - .borrow_mut() - .data_as_mut_slice() - .get_mut(0) - .unwrap() = 0; - - invoke_context.pop().unwrap(); - } - - // Internal modification tests + // Account modification tests let cases = vec![ (MockInstruction::NoopSuccess, Ok(())), ( @@ -1574,6 +1479,14 @@ mod tests { MockInstruction::ModifyReadonly, Err(InstructionError::ReadonlyDataModified), ), + ( + MockInstruction::UnbalancedPush, + Err(InstructionError::UnbalancedInstruction), + ), + ( + MockInstruction::UnbalancedPop, + Err(InstructionError::UnbalancedInstruction), + ), ]; for case in cases { invoke_context @@ -1581,8 +1494,10 @@ mod tests { .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().unwrap(); + let result = invoke_context + .native_invoke(inner_instruction, &[]) + .and(invoke_context.pop()); + assert_eq!(result, case.1); } // Compute unit consumption tests @@ -1628,7 +1543,8 @@ mod tests { fn test_invoke_context_compute_budget() { let accounts = vec![(solana_sdk::pubkey::new_rand(), AccountSharedData::default())]; - let mut transaction_context = TransactionContext::new(accounts, 1, 3); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 1, 3); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); invoke_context.compute_budget = ComputeBudget::new(compute_budget::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64); @@ -1663,7 +1579,8 @@ mod tests { process_instruction: mock_process_instruction, }]; - let mut transaction_context = TransactionContext::new(accounts, 1, 3); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 1, 3); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &builtin_programs); @@ -1707,7 +1624,13 @@ mod tests { ); assert!(result.is_ok()); - assert_eq!(invoke_context.accounts_data_meter.remaining(), 0); + assert_eq!( + invoke_context + .transaction_context + .accounts_resize_delta() + .unwrap(), + user_account_data_len as i64 * 2 + ); } // Test 2: Resize the account to *the same size*, so not consuming any additional size; this must succeed @@ -1725,10 +1648,16 @@ mod tests { ); assert!(result.is_ok()); - assert_eq!(invoke_context.accounts_data_meter.remaining(), 0); + assert_eq!( + invoke_context + .transaction_context + .accounts_resize_delta() + .unwrap(), + user_account_data_len as i64 * 2 + ); } - // Test 3: Resize the account to exceed the budget; this must fail + // Test 3: Resize the account to exceed the budget; this must succeed { let new_len = user_account_data_len + remaining_account_data_len + 1; let instruction_data = @@ -1742,12 +1671,14 @@ mod tests { &mut ExecuteTimings::default(), ); - assert!(result.is_err()); - assert!(matches!( - result, - Err(solana_sdk::instruction::InstructionError::MaxAccountsDataSizeExceeded) - )); - assert_eq!(invoke_context.accounts_data_meter.remaining(), 0); + assert!(result.is_ok()); + assert_eq!( + invoke_context + .transaction_context + .accounts_resize_delta() + .unwrap(), + user_account_data_len as i64 * 2 + 1 + ); } } } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 8df3c9ac1..acfcf1f45 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -183,8 +183,14 @@ pub fn builtin_process_instruction( if borrowed_account.get_lamports() != lamports { borrowed_account.set_lamports(lamports)?; } - if borrowed_account.get_data() != data { - borrowed_account.set_data(&data)?; + // The redundant check helps to avoid the expensive data comparison if we can + match borrowed_account + .can_data_be_resized(data.len()) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => borrowed_account.set_data(&data)?, + Err(err) if borrowed_account.get_data() != data => return Err(err), + _ => {} } if borrowed_account.get_owner() != &owner { borrowed_account.set_owner(owner.as_ref())?; @@ -302,14 +308,23 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { .unwrap(); } let account_info_data = account_info.try_borrow_data().unwrap(); - if borrowed_account.get_data() != *account_info_data { - borrowed_account.set_data(&account_info_data).unwrap(); + // The redundant check helps to avoid the expensive data comparison if we can + match borrowed_account + .can_data_be_resized(account_info_data.len()) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => borrowed_account.set_data(&account_info_data).unwrap(), + Err(err) if borrowed_account.get_data() != *account_info_data => { + panic!("{:?}", err); + } + _ => {} } if borrowed_account.is_executable() != account_info.executable { borrowed_account .set_executable(account_info.executable) .unwrap(); } + // Change the owner at the end so that we are allowed to change the lamports and data before if borrowed_account.get_owner() != account_info.owner { borrowed_account .set_owner(account_info.owner.as_ref()) diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 0439316dc..216c38574 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -305,7 +305,7 @@ fn run_program(name: &str) -> u64 { tracer = Some(vm.get_tracer().clone()); } } - deserialize_parameters( + assert!(match deserialize_parameters( invoke_context.transaction_context, invoke_context .transaction_context @@ -313,8 +313,21 @@ fn run_program(name: &str) -> u64 { .unwrap(), parameter_bytes.as_slice(), &account_lengths, - ) - .unwrap(); + ) { + Ok(()) => true, + Err(InstructionError::ModifiedProgramId) => true, + Err(InstructionError::ExternalAccountLamportSpend) => true, + Err(InstructionError::ReadonlyLamportChange) => true, + Err(InstructionError::ExecutableLamportChange) => true, + Err(InstructionError::ExecutableAccountNotRentExempt) => true, + Err(InstructionError::ExecutableModified) => true, + Err(InstructionError::AccountDataSizeChanged) => true, + Err(InstructionError::InvalidRealloc) => true, + Err(InstructionError::ExecutableDataModified) => true, + Err(InstructionError::ReadonlyDataModified) => true, + Err(InstructionError::ExternalAccountDataModified) => true, + _ => false, + }); } instruction_count }) diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 2be8073bf..7d6639453 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -9,6 +9,7 @@ use { solana_sdk::{ account::{Account, AccountSharedData}, bpf_loader, + sysvar::rent::Rent, transaction_context::{InstructionAccount, TransactionContext}, }, test::Bencher, @@ -101,7 +102,8 @@ fn create_inputs() -> TransactionContext { }, ) .collect::>(); - let mut transaction_context = TransactionContext::new(transaction_accounts, 1, 1); + let mut transaction_context = + TransactionContext::new(transaction_accounts, Some(Rent::default()), 1, 1); let instruction_data = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; transaction_context .push(&[0], &instruction_accounts, &instruction_data, true) diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index 6f3f0add6..2aa5acd57 100644 --- a/programs/bpf_loader/src/serialization.rs +++ b/programs/bpf_loader/src/serialization.rs @@ -164,18 +164,28 @@ pub fn deserialize_parameters_unaligned( start += size_of::(); // is_signer start += size_of::(); // is_writable start += size_of::(); // key - let _ = borrowed_account.set_lamports(LittleEndian::read_u64( + let lamports = LittleEndian::read_u64( buffer .get(start..) .ok_or(InstructionError::InvalidArgument)?, - )); + ); + if borrowed_account.get_lamports() != lamports { + borrowed_account.set_lamports(lamports)?; + } start += size_of::() // lamports + size_of::(); // data length - let _ = borrowed_account.set_data( - buffer - .get(start..start + pre_len) - .ok_or(InstructionError::InvalidArgument)?, - ); + let data = buffer + .get(start..start + pre_len) + .ok_or(InstructionError::InvalidArgument)?; + // The redundant check helps to avoid the expensive data comparison if we can + match borrowed_account + .can_data_be_resized(data.len()) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => borrowed_account.set_data(data)?, + Err(err) if borrowed_account.get_data() != data => return Err(err), + _ => {} + } start += pre_len // data + size_of::() // owner + size_of::() // executable @@ -302,17 +312,18 @@ pub fn deserialize_parameters_aligned( + size_of::() // executable + size_of::() // original_data_len + size_of::(); // key - let _ = borrowed_account.set_owner( - buffer - .get(start..start + size_of::()) - .ok_or(InstructionError::InvalidArgument)?, - ); + let owner = buffer + .get(start..start + size_of::()) + .ok_or(InstructionError::InvalidArgument)?; start += size_of::(); // owner - let _ = borrowed_account.set_lamports(LittleEndian::read_u64( + let lamports = LittleEndian::read_u64( buffer .get(start..) .ok_or(InstructionError::InvalidArgument)?, - )); + ); + if borrowed_account.get_lamports() != lamports { + borrowed_account.set_lamports(lamports)?; + } start += size_of::(); // lamports let post_len = LittleEndian::read_u64( buffer @@ -320,21 +331,31 @@ pub fn deserialize_parameters_aligned( .ok_or(InstructionError::InvalidArgument)?, ) as usize; start += size_of::(); // data length - if post_len.saturating_sub(*pre_len) > MAX_PERMITTED_DATA_INCREASE || post_len > MAX_PERMITTED_DATA_LENGTH as usize { return Err(InstructionError::InvalidRealloc); } let data_end = start + post_len; - let _ = borrowed_account.set_data( - buffer - .get(start..data_end) - .ok_or(InstructionError::InvalidArgument)?, - ); + let data = buffer + .get(start..data_end) + .ok_or(InstructionError::InvalidArgument)?; + // The redundant check helps to avoid the expensive data comparison if we can + match borrowed_account + .can_data_be_resized(data.len()) + .and_then(|_| borrowed_account.can_data_be_changed()) + { + Ok(()) => borrowed_account.set_data(data)?, + Err(err) if borrowed_account.get_data() != data => return Err(err), + _ => {} + } start += *pre_len + MAX_PERMITTED_DATA_INCREASE; // data start += (start as *const u8).align_offset(BPF_ALIGN_OF_U128); start += size_of::(); // rent_epoch + if borrowed_account.get_owner().to_bytes() != owner { + // Change the owner at the end so that we are allowed to change the lamports and data before + borrowed_account.set_owner(owner)?; + } } } Ok(()) @@ -351,6 +372,7 @@ mod tests { bpf_loader, entrypoint::deserialize, instruction::AccountMeta, + sysvar::rent::Rent, }, std::{ cell::RefCell, @@ -453,8 +475,12 @@ mod tests { instruction_accounts, &program_indices, ); - let mut transaction_context = - TransactionContext::new(preparation.transaction_accounts, 1, 1); + let mut transaction_context = TransactionContext::new( + preparation.transaction_accounts, + Some(Rent::default()), + 1, + 1, + ); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); invoke_context .push( @@ -512,16 +538,6 @@ mod tests { ); } - for index_in_transaction in 1..original_accounts.len() { - let mut account = invoke_context - .transaction_context - .get_account_at_index(index_in_transaction) - .unwrap() - .borrow_mut(); - account.set_lamports(0); - account.set_data(vec![0; 0]); - account.set_owner(Pubkey::default()); - } deserialize_parameters( invoke_context.transaction_context, instruction_context, @@ -545,13 +561,12 @@ mod tests { .unwrap() .1 .set_owner(bpf_loader_deprecated::id()); - let _ = invoke_context + invoke_context .transaction_context - .get_current_instruction_context() + .get_account_at_index(0) .unwrap() - .try_borrow_program_account(invoke_context.transaction_context, 0) - .unwrap() - .set_owner(bpf_loader_deprecated::id().as_ref()); + .borrow_mut() + .set_owner(bpf_loader_deprecated::id()); let (mut serialized, account_lengths) = serialize_parameters(invoke_context.transaction_context, instruction_context).unwrap(); @@ -578,15 +593,6 @@ mod tests { assert_eq!(account.rent_epoch(), account_info.rent_epoch); } - for index_in_transaction in 1..original_accounts.len() { - let mut account = invoke_context - .transaction_context - .get_account_at_index(index_in_transaction) - .unwrap() - .borrow_mut(); - account.set_lamports(0); - account.set_data(vec![0; 0]); - } deserialize_parameters( invoke_context.transaction_context, instruction_context, diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 23ab57b7f..02b603f4b 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -17,13 +17,14 @@ use { vm::{EbpfVm, SyscallObject, SyscallRegistry}, }, solana_sdk::{ - account::WritableAccount, + account::{ReadableAccount, WritableAccount}, account_info::AccountInfo, blake3, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS}, feature_set::{ blake3_syscall_enabled, check_physical_overlapping, check_slice_translation_size, - curve25519_syscall_enabled, disable_fees_sysvar, libsecp256k1_0_5_upgrade_enabled, + curve25519_syscall_enabled, disable_fees_sysvar, + enable_early_verification_of_account_modifications, libsecp256k1_0_5_upgrade_enabled, limit_secp256k1_recovery_id, prevent_calling_precompiles_as_programs, syscall_saturated_math, }, @@ -2663,26 +2664,56 @@ where invoke_context, )?; { - callee_account - .set_lamports(*caller_account.lamports) - .map_err(SyscallError::InstructionError)?; - callee_account - .set_data(caller_account.data) - .map_err(SyscallError::InstructionError)?; - callee_account - .set_executable(caller_account.executable) - .map_err(SyscallError::InstructionError)?; - callee_account - .set_owner(caller_account.owner.as_ref()) - .map_err(SyscallError::InstructionError)?; + if callee_account.get_lamports() != *caller_account.lamports { + callee_account + .set_lamports(*caller_account.lamports) + .map_err(SyscallError::InstructionError)?; + } + // The redundant check helps to avoid the expensive data comparison if we can + match callee_account + .can_data_be_resized(caller_account.data.len()) + .and_then(|_| callee_account.can_data_be_changed()) + { + Ok(()) => callee_account + .set_data(caller_account.data) + .map_err(SyscallError::InstructionError)?, + Err(err) if callee_account.get_data() != caller_account.data => { + return Err(EbpfError::UserError(BpfError::SyscallError( + SyscallError::InstructionError(err), + ))); + } + _ => {} + } + if callee_account.is_executable() != caller_account.executable { + callee_account + .set_executable(caller_account.executable) + .map_err(SyscallError::InstructionError)?; + } + // Change the owner at the end so that we are allowed to change the lamports and data before + if callee_account.get_owner() != caller_account.owner { + callee_account + .set_owner(caller_account.owner.as_ref()) + .map_err(SyscallError::InstructionError)?; + } drop(callee_account); let callee_account = invoke_context .transaction_context .get_account_at_index(instruction_account.index_in_transaction) .map_err(SyscallError::InstructionError)?; - callee_account - .borrow_mut() - .set_rent_epoch(caller_account.rent_epoch); + if callee_account.borrow().rent_epoch() != caller_account.rent_epoch { + if invoke_context + .feature_set + .is_active(&enable_early_verification_of_account_modifications::id()) + { + Err(SyscallError::InstructionError( + InstructionError::RentEpochModified, + ))?; + } else { + callee_account + .borrow_mut() + .set_rent_epoch(caller_account.rent_epoch); + } + } } let caller_account = if instruction_account.is_writable { let orig_data_lens = invoke_context @@ -3364,7 +3395,8 @@ mod tests { ), ($program_key, AccountSharedData::new(0, 0, &$loader_key)), ]; - let mut $transaction_context = TransactionContext::new(transaction_accounts, 1, 1); + let mut $transaction_context = + TransactionContext::new(transaction_accounts, Some(Rent::default()), 1, 1); let mut $invoke_context = InvokeContext::new_mock(&mut $transaction_context, &[]); $invoke_context.push(&[], &[0, 1], &[]).unwrap(); }; diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 539dc8315..ddd87cd3c 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -2783,6 +2783,7 @@ mod tests { Rent::id(), create_account_shared_data_for_test(&Rent::default()), )], + Some(Rent::default()), 1, 1, ) @@ -2894,7 +2895,8 @@ mod tests { #[test] fn test_things_can_merge() { - let mut transaction_context = TransactionContext::new(Vec::new(), 1, 1); + let mut transaction_context = + TransactionContext::new(Vec::new(), Some(Rent::default()), 1, 1); let invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); let good_stake = Stake { credits_observed: 4242, @@ -2993,7 +2995,8 @@ mod tests { #[test] fn test_metas_can_merge() { - let mut transaction_context = TransactionContext::new(Vec::new(), 1, 1); + let mut transaction_context = + TransactionContext::new(Vec::new(), Some(Rent::default()), 1, 1); let invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); // Identical Metas can merge assert!(MergeKind::metas_can_merge( @@ -3140,7 +3143,8 @@ mod tests { #[test] fn test_merge_kind_get_if_mergeable() { - let mut transaction_context = TransactionContext::new(Vec::new(), 1, 1); + let mut transaction_context = + TransactionContext::new(Vec::new(), Some(Rent::default()), 1, 1); let invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); let authority_pubkey = Pubkey::new_unique(); let initial_lamports = 4242424242; @@ -3379,7 +3383,8 @@ mod tests { #[test] fn test_merge_kind_merge() { - let mut transaction_context = TransactionContext::new(Vec::new(), 1, 1); + let mut transaction_context = + TransactionContext::new(Vec::new(), Some(Rent::default()), 1, 1); let invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); let clock = Clock::default(); let lamports = 424242; diff --git a/programs/vote/benches/process_vote.rs b/programs/vote/benches/process_vote.rs index f939c6c26..5cce571ec 100644 --- a/programs/vote/benches/process_vote.rs +++ b/programs/vote/benches/process_vote.rs @@ -107,7 +107,12 @@ fn bench_process_vote_instruction( instruction_data: Vec, ) { bencher.iter(|| { - let mut transaction_context = TransactionContext::new(transaction_accounts.clone(), 1, 1); + let mut transaction_context = TransactionContext::new( + transaction_accounts.clone(), + Some(sysvar::rent::Rent::default()), + 1, + 1, + ); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); invoke_context .push(&instruction_accounts, &[0], &instruction_data) diff --git a/rbpf-cli/src/main.rs b/rbpf-cli/src/main.rs index 13601e549..1e772ce4e 100644 --- a/rbpf-cli/src/main.rs +++ b/rbpf-cli/src/main.rs @@ -16,7 +16,7 @@ use { }, solana_sdk::{ account::AccountSharedData, bpf_loader, instruction::AccountMeta, pubkey::Pubkey, - transaction_context::TransactionContext, + sysvar::rent::Rent, transaction_context::TransactionContext, }, std::{ fmt::{Debug, Formatter}, @@ -216,7 +216,12 @@ native machine code before execting it in the virtual machine.", let program_indices = [0, 1]; let preparation = prepare_mock_invoke_context(transaction_accounts, instruction_accounts, &program_indices); - let mut transaction_context = TransactionContext::new(preparation.transaction_accounts, 1, 1); + let mut transaction_context = TransactionContext::new( + preparation.transaction_accounts, + Some(Rent::default()), + 1, + 1, + ); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); invoke_context .push( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 29693b6e2..0f5437842 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -109,7 +109,7 @@ use { feature, feature_set::{ self, add_set_compute_unit_price_ix, default_units_per_instruction, - disable_fee_calculator, FeatureSet, + disable_fee_calculator, enable_early_verification_of_account_modifications, FeatureSet, }, fee::FeeStructure, fee_calculator::{FeeCalculator, FeeRateGovernor}, @@ -4274,6 +4274,14 @@ impl Bank { let transaction_accounts = std::mem::take(&mut loaded_transaction.accounts); let mut transaction_context = TransactionContext::new( transaction_accounts, + if self + .feature_set + .is_active(&enable_early_verification_of_account_modifications::id()) + { + Some(self.rent_collector.rent) + } else { + None + }, compute_budget.max_invoke_depth.saturating_add(1), tx.message().instructions().len(), ); @@ -4352,7 +4360,7 @@ impl Bank { } err }); - let accounts_data_len_delta = status + let mut accounts_data_len_delta = status .as_ref() .map_or(0, |info| info.accounts_data_len_delta); let status = status.map(|_| ()); @@ -4368,9 +4376,31 @@ impl Bank { accounts, instruction_trace, mut return_data, - .. + changed_account_count, + total_size_of_all_accounts, + total_size_of_touched_accounts, + accounts_resize_delta, } = transaction_context.into(); loaded_transaction.accounts = accounts; + if self + .feature_set + .is_active(&enable_early_verification_of_account_modifications::id()) + { + saturating_add_assign!( + timings.details.total_account_count, + loaded_transaction.accounts.len() as u64 + ); + saturating_add_assign!(timings.details.changed_account_count, changed_account_count); + saturating_add_assign!( + timings.details.total_data_size, + total_size_of_all_accounts as usize + ); + saturating_add_assign!( + timings.details.data_size_changed, + total_size_of_touched_accounts as usize + ); + 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( @@ -18668,6 +18698,7 @@ pub(crate) mod tests { }); let transaction_context = TransactionContext::new( loaded_txs[0].0.as_ref().unwrap().accounts.clone(), + Some(Rent::default()), compute_budget.max_invoke_depth.saturating_add(1), number_of_instructions_at_transaction_level, ); @@ -18819,15 +18850,15 @@ pub(crate) mod tests { fn test_inner_instructions_list_from_instruction_trace() { let instruction_trace = vec![ vec![ - InstructionContext::new(0, &[], &[], &[1]), - InstructionContext::new(1, &[], &[], &[2]), + InstructionContext::new(0, 0, &[], &[], &[1]), + InstructionContext::new(1, 0, &[], &[], &[2]), ], vec![], vec![ - InstructionContext::new(0, &[], &[], &[3]), - InstructionContext::new(1, &[], &[], &[4]), - InstructionContext::new(2, &[], &[], &[5]), - InstructionContext::new(1, &[], &[], &[6]), + InstructionContext::new(0, 0, &[], &[], &[3]), + InstructionContext::new(1, 0, &[], &[], &[4]), + InstructionContext::new(2, 0, &[], &[], &[5]), + InstructionContext::new(1, 0, &[], &[], &[6]), ], ]; diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index d3e760de2..8afa00950 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -282,7 +282,8 @@ mod tests { create_loadable_account_for_test("mock_system_program"), ), ]; - let mut transaction_context = TransactionContext::new(accounts, 1, 3); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 1, 3); let program_indices = vec![vec![2]]; let executors = Rc::new(RefCell::new(Executors::default())); let account_keys = transaction_context.get_keys_of_accounts().to_vec(); @@ -502,7 +503,8 @@ mod tests { create_loadable_account_for_test("mock_system_program"), ), ]; - let mut transaction_context = TransactionContext::new(accounts, 1, 3); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 1, 3); let program_indices = vec![vec![2]]; let executors = Rc::new(RefCell::new(Executors::default())); let account_metas = vec![ @@ -661,7 +663,8 @@ mod tests { (secp256k1_program::id(), secp256k1_account), (mock_program_id, mock_program_account), ]; - let mut transaction_context = TransactionContext::new(accounts, 1, 2); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 1, 2); let message = SanitizedMessage::Legacy(Message::new( &[ diff --git a/runtime/src/nonce_keyed_account.rs b/runtime/src/nonce_keyed_account.rs index a129b4505..5e6537a3f 100644 --- a/runtime/src/nonce_keyed_account.rs +++ b/runtime/src/nonce_keyed_account.rs @@ -328,7 +328,8 @@ mod test { is_writable: true, }, ]; - let mut transaction_context = TransactionContext::new(accounts, 1, 2); + let mut transaction_context = + TransactionContext::new(accounts, Some(Rent::default()), 1, 2); let mut $invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); }; } diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index 2eb7621cf..67f1f9311 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -786,7 +786,8 @@ mod tests { #[test] fn test_address_create_with_seed_mismatch() { - let mut transaction_context = TransactionContext::new(Vec::new(), 1, 1); + let mut transaction_context = + TransactionContext::new(Vec::new(), Some(Rent::default()), 1, 1); let invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); let from = Pubkey::new_unique(); let seed = "dull boy"; diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index cdbe902bc..7828db99b 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -456,6 +456,10 @@ pub mod enable_bpf_loader_extend_program_data_ix { solana_sdk::declare_id!("8Zs9W7D9MpSEtUWSQdGniZk2cNmV22y6FLJwCx53asme"); } +pub mod enable_early_verification_of_account_modifications { + solana_sdk::declare_id!("7Vced912WrRnfjaiKRiNBcbuFw7RrnLv3E3z95Y4GTNc"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -564,6 +568,7 @@ lazy_static! { (cap_accounts_data_size_per_block::id(), "cap the accounts data size per block #25517"), (preserve_rent_epoch_for_rent_exempt_accounts::id(), "preserve rent epoch for rent exempt accounts #26479"), (enable_bpf_loader_extend_program_data_ix::id(), "enable bpf upgradeable loader ExtendProgramData instruction #25234"), + (enable_early_verification_of_account_modifications::id(), "enable early verification of account modifications #25899"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 51bc4c726..ba1eba71c 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -5,6 +5,8 @@ use { account::{AccountSharedData, ReadableAccount, WritableAccount}, instruction::InstructionError, pubkey::Pubkey, + rent::Rent, + system_instruction::MAX_PERMITTED_DATA_LENGTH, }, std::{ cell::{RefCell, RefMut}, @@ -43,18 +45,21 @@ pub struct InstructionAccount { pub struct TransactionContext { account_keys: Pin>, accounts: Pin]>>, + account_touched_flags: RefCell>>, instruction_context_capacity: usize, instruction_stack: Vec, number_of_instructions_at_transaction_level: usize, instruction_trace: InstructionTrace, return_data: TransactionReturnData, - total_resize_delta: RefCell, + accounts_resize_delta: RefCell, + rent: Option, } impl TransactionContext { /// Constructs a new TransactionContext pub fn new( transaction_accounts: Vec, + rent: Option, instruction_context_capacity: usize, number_of_instructions_at_transaction_level: usize, ) -> Self { @@ -63,15 +68,18 @@ impl TransactionContext { .into_iter() .map(|(key, account)| (key, RefCell::new(account))) .unzip(); + let account_touched_flags = vec![false; accounts.len()]; Self { account_keys: Pin::new(account_keys.into_boxed_slice()), accounts: Pin::new(accounts.into_boxed_slice()), + account_touched_flags: RefCell::new(Pin::new(account_touched_flags.into_boxed_slice())), instruction_context_capacity, instruction_stack: Vec::with_capacity(instruction_context_capacity), number_of_instructions_at_transaction_level, instruction_trace: Vec::with_capacity(number_of_instructions_at_transaction_level), return_data: TransactionReturnData::default(), - total_resize_delta: RefCell::new(0), + accounts_resize_delta: RefCell::new(0), + rent, } } @@ -86,6 +94,11 @@ impl TransactionContext { .collect()) } + /// Returns true if `enable_early_verification_of_account_modifications` is active + pub fn is_early_verification_of_account_modifications_enabled(&self) -> bool { + self.rent.is_some() + } + /// Returns the total number of accounts loaded in this Transaction pub fn get_number_of_accounts(&self) -> usize { self.accounts.len() @@ -180,31 +193,51 @@ impl TransactionContext { instruction_data: &[u8], record_instruction_in_transaction_context_push: bool, ) -> Result<(), InstructionError> { + let callee_instruction_accounts_lamport_sum = + self.instruction_accounts_lamport_sum(instruction_accounts)?; let index_in_trace = if self.instruction_stack.is_empty() { debug_assert!( self.instruction_trace.len() < self.number_of_instructions_at_transaction_level ); let instruction_context = InstructionContext { nesting_level: self.instruction_stack.len(), + instruction_accounts_lamport_sum: callee_instruction_accounts_lamport_sum, program_accounts: program_accounts.to_vec(), instruction_accounts: instruction_accounts.to_vec(), instruction_data: instruction_data.to_vec(), }; self.instruction_trace.push(vec![instruction_context]); self.instruction_trace.len().saturating_sub(1) - } else if let Some(instruction_trace) = self.instruction_trace.last_mut() { - if record_instruction_in_transaction_context_push { - let instruction_context = InstructionContext { - nesting_level: self.instruction_stack.len(), - program_accounts: program_accounts.to_vec(), - instruction_accounts: instruction_accounts.to_vec(), - instruction_data: instruction_data.to_vec(), - }; - instruction_trace.push(instruction_context); - } - instruction_trace.len().saturating_sub(1) } else { - return Err(InstructionError::CallDepth); + if self.is_early_verification_of_account_modifications_enabled() { + let caller_instruction_context = self.get_current_instruction_context()?; + let original_caller_instruction_accounts_lamport_sum = + caller_instruction_context.instruction_accounts_lamport_sum; + let current_caller_instruction_accounts_lamport_sum = self + .instruction_accounts_lamport_sum( + &caller_instruction_context.instruction_accounts, + )?; + if original_caller_instruction_accounts_lamport_sum + != current_caller_instruction_accounts_lamport_sum + { + return Err(InstructionError::UnbalancedInstruction); + } + } + if let Some(instruction_trace) = self.instruction_trace.last_mut() { + if record_instruction_in_transaction_context_push { + let instruction_context = InstructionContext { + nesting_level: self.instruction_stack.len(), + instruction_accounts_lamport_sum: callee_instruction_accounts_lamport_sum, + program_accounts: program_accounts.to_vec(), + instruction_accounts: instruction_accounts.to_vec(), + instruction_data: instruction_data.to_vec(), + }; + instruction_trace.push(instruction_context); + } + instruction_trace.len().saturating_sub(1) + } else { + return Err(InstructionError::CallDepth); + } }; if self.instruction_stack.len() >= self.instruction_context_capacity { return Err(InstructionError::CallDepth); @@ -218,8 +251,34 @@ impl TransactionContext { if self.instruction_stack.is_empty() { 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) + .map(|instruction_accounts_lamport_sum| { + instruction_context.instruction_accounts_lamport_sum + != instruction_accounts_lamport_sum + }) + }) + } else { + Ok(false) + }; + // Always pop, even if we `detected_an_unbalanced_instruction` self.instruction_stack.pop(); - Ok(()) + if detected_an_unbalanced_instruction? { + Err(InstructionError::UnbalancedInstruction) + } else { + Ok(()) + } } /// Gets the return data of the current InstructionContext or any above @@ -251,6 +310,40 @@ impl TransactionContext { pub fn get_instruction_trace(&self) -> &InstructionTrace { &self.instruction_trace } + + /// Calculates the sum of all lamports within an instruction + pub fn instruction_accounts_lamport_sum( + &self, + instruction_accounts: &[InstructionAccount], + ) -> Result { + if !self.is_early_verification_of_account_modifications_enabled() { + return Ok(0); + } + let mut instruction_accounts_lamport_sum: u128 = 0; + for (instruction_account_index, instruction_account) in + instruction_accounts.iter().enumerate() + { + if instruction_account_index != instruction_account.index_in_callee { + continue; // Skip duplicate account + } + instruction_accounts_lamport_sum = (self + .get_account_at_index(instruction_account.index_in_transaction)? + .try_borrow() + .map_err(|_| InstructionError::AccountBorrowOutstanding)? + .lamports() as u128) + .checked_add(instruction_accounts_lamport_sum) + .ok_or(InstructionError::ArithmeticOverflow)?; + } + Ok(instruction_accounts_lamport_sum) + } + + /// Returns the accounts resize delta + pub fn accounts_resize_delta(&self) -> Result { + self.accounts_resize_delta + .try_borrow() + .map_err(|_| InstructionError::GenericError) + .map(|value_ref| *value_ref) + } } /// Return data at the end of a transaction @@ -269,6 +362,7 @@ pub type InstructionTrace = Vec>; #[derive(Debug, Clone)] pub struct InstructionContext { nesting_level: usize, + instruction_accounts_lamport_sum: u128, program_accounts: Vec, instruction_accounts: Vec, instruction_data: Vec, @@ -278,12 +372,14 @@ impl InstructionContext { /// New pub fn new( nesting_level: usize, + instruction_accounts_lamport_sum: u128, program_accounts: &[usize], instruction_accounts: &[InstructionAccount], instruction_data: &[u8], ) -> Self { InstructionContext { nesting_level, + instruction_accounts_lamport_sum, program_accounts: program_accounts.to_vec(), instruction_accounts: instruction_accounts.to_vec(), instruction_data: instruction_data.to_vec(), @@ -540,6 +636,32 @@ impl<'a> BorrowedAccount<'a> { /// Assignes the owner of this account (transaction wide) pub fn set_owner(&mut self, pubkey: &[u8]) -> Result<(), InstructionError> { + if self + .transaction_context + .is_early_verification_of_account_modifications_enabled() + { + // Only the owner can assign a new owner + if !self.is_owned_by_current_program() { + return Err(InstructionError::ModifiedProgramId); + } + // and only if the account is writable + if !self.is_writable() { + return Err(InstructionError::ModifiedProgramId); + } + // and only if the account is not executable + if self.is_executable() { + return Err(InstructionError::ModifiedProgramId); + } + // and only if the data is zero-initialized or empty + if !is_zeroed(self.get_data()) { + return Err(InstructionError::ModifiedProgramId); + } + // don't touch the account if the owner does not change + if self.get_owner().to_bytes() == pubkey { + return Ok(()); + } + self.touch()?; + } self.account.copy_into_owner_from_slice(pubkey); Ok(()) } @@ -551,6 +673,28 @@ impl<'a> BorrowedAccount<'a> { /// Overwrites the number of lamports of this account (transaction wide) pub fn set_lamports(&mut self, lamports: u64) -> Result<(), InstructionError> { + if self + .transaction_context + .is_early_verification_of_account_modifications_enabled() + { + // An account not owned by the program cannot have its balance decrease + if !self.is_owned_by_current_program() && lamports < self.get_lamports() { + return Err(InstructionError::ExternalAccountLamportSpend); + } + // The balance of read-only may not change + if !self.is_writable() { + return Err(InstructionError::ReadonlyLamportChange); + } + // The balance of executable accounts may not change + if self.is_executable() { + return Err(InstructionError::ExecutableLamportChange); + } + // don't touch the account if the lamports do not change + if self.get_lamports() == lamports { + return Ok(()); + } + self.touch()?; + } self.account.set_lamports(lamports); Ok(()) } @@ -580,16 +724,25 @@ impl<'a> BorrowedAccount<'a> { /// Returns a writable slice of the account data (transaction wide) pub fn get_data_mut(&mut self) -> Result<&mut [u8], InstructionError> { + self.can_data_be_changed()?; + self.touch()?; Ok(self.account.data_as_mut_slice()) } /// Overwrites the account data and size (transaction wide) pub fn set_data(&mut self, data: &[u8]) -> Result<(), InstructionError> { + self.can_data_be_resized(data.len())?; + self.can_data_be_changed()?; + self.touch()?; if data.len() == self.account.data().len() { self.account.data_as_mut_slice().copy_from_slice(data); } else { - let mut total_resize_delta = self.transaction_context.total_resize_delta.borrow_mut(); - *total_resize_delta = total_resize_delta + let mut accounts_resize_delta = self + .transaction_context + .accounts_resize_delta + .try_borrow_mut() + .map_err(|_| InstructionError::GenericError)?; + *accounts_resize_delta = accounts_resize_delta .saturating_add((data.len() as i64).saturating_sub(self.get_data().len() as i64)); self.account.set_data_from_slice(data); } @@ -600,8 +753,19 @@ impl<'a> BorrowedAccount<'a> { /// /// Fills it with zeros at the end if is extended or truncates at the end otherwise. pub fn set_data_length(&mut self, new_length: usize) -> Result<(), InstructionError> { - let mut total_resize_delta = self.transaction_context.total_resize_delta.borrow_mut(); - *total_resize_delta = total_resize_delta + self.can_data_be_resized(new_length)?; + self.can_data_be_changed()?; + // don't touch the account if the length does not change + if self.get_data().len() == new_length { + return Ok(()); + } + self.touch()?; + let mut accounts_resize_delta = self + .transaction_context + .accounts_resize_delta + .try_borrow_mut() + .map_err(|_| InstructionError::GenericError)?; + *accounts_resize_delta = accounts_resize_delta .saturating_add((new_length as i64).saturating_sub(self.get_data().len() as i64)); self.account.data_mut().resize(new_length, 0); Ok(()) @@ -616,7 +780,7 @@ impl<'a> BorrowedAccount<'a> { /// Serializes a state into the account data pub fn set_state(&mut self, state: &T) -> Result<(), InstructionError> { - let data = self.account.data_as_mut_slice(); + let data = self.get_data_mut()?; let serialized_size = bincode::serialized_size(state).map_err(|_| InstructionError::GenericError)?; if serialized_size > data.len() as u64 { @@ -633,6 +797,29 @@ impl<'a> BorrowedAccount<'a> { /// Configures whether this account is executable (transaction wide) pub fn set_executable(&mut self, is_executable: bool) -> Result<(), InstructionError> { + if let Some(rent) = self.transaction_context.rent { + // To become executable an account must be rent exempt + if !rent.is_exempt(self.get_lamports(), self.get_data().len()) { + return Err(InstructionError::ExecutableAccountNotRentExempt); + } + // Only the owner can set the executable flag + if !self.is_owned_by_current_program() { + return Err(InstructionError::ExecutableModified); + } + // and only if the account is writable + if !self.is_writable() { + return Err(InstructionError::ExecutableModified); + } + // one can not clear the executable flag + if self.is_executable() && !is_executable { + return Err(InstructionError::ExecutableModified); + } + // don't touch the account if the executable flag does not change + if self.is_executable() == is_executable { + return Ok(()); + } + self.touch()?; + } self.account.set_executable(is_executable); Ok(()) } @@ -667,6 +854,72 @@ impl<'a> BorrowedAccount<'a> { ) .unwrap_or_default() } + + /// Returns true if the owner of this account is the current `InstructionContext`s last program (instruction wide) + pub fn is_owned_by_current_program(&self) -> bool { + self.instruction_context + .get_last_program_key(self.transaction_context) + .map(|key| key == self.get_owner()) + .unwrap_or_default() + } + + /// Returns an error if the account data can not be mutated by the current program + pub fn can_data_be_changed(&self) -> Result<(), InstructionError> { + if !self + .transaction_context + .is_early_verification_of_account_modifications_enabled() + { + return Ok(()); + } + // Only non-executable accounts data can be changed + if self.is_executable() { + return Err(InstructionError::ExecutableDataModified); + } + // and only if the account is writable + if !self.is_writable() { + return Err(InstructionError::ReadonlyDataModified); + } + // and only if we are the owner + if !self.is_owned_by_current_program() { + return Err(InstructionError::ExternalAccountDataModified); + } + Ok(()) + } + + /// Returns an error if the account data can not be resized to the given length + pub fn can_data_be_resized(&self, new_length: usize) -> Result<(), InstructionError> { + if !self + .transaction_context + .is_early_verification_of_account_modifications_enabled() + { + return Ok(()); + } + // Only the owner can change the length of the data + if new_length != self.get_data().len() && !self.is_owned_by_current_program() { + return Err(InstructionError::AccountDataSizeChanged); + } + // The new length can not exceed the maximum permitted length + if new_length > MAX_PERMITTED_DATA_LENGTH as usize { + return Err(InstructionError::InvalidRealloc); + } + Ok(()) + } + + fn touch(&self) -> Result<(), InstructionError> { + if self + .transaction_context + .is_early_verification_of_account_modifications_enabled() + { + *self + .transaction_context + .account_touched_flags + .try_borrow_mut() + .map_err(|_| InstructionError::GenericError)? + .get_mut(self.index_in_transaction) + .ok_or(InstructionError::NotEnoughAccountKeys)? = true; + } + Ok(()) + } } /// Everything that needs to be recorded from a TransactionContext after execution @@ -674,12 +927,38 @@ pub struct ExecutionRecord { pub accounts: Vec, pub instruction_trace: InstructionTrace, pub return_data: TransactionReturnData, - pub total_resize_delta: i64, + pub changed_account_count: u64, + pub total_size_of_all_accounts: u64, + pub total_size_of_touched_accounts: u64, + pub accounts_resize_delta: i64, } /// Used by the bank in the runtime to write back the processed accounts and recorded instructions impl From for ExecutionRecord { fn from(context: TransactionContext) -> Self { + let mut changed_account_count = 0u64; + let mut total_size_of_all_accounts = 0u64; + let mut total_size_of_touched_accounts = 0u64; + let account_touched_flags = context + .account_touched_flags + .try_borrow() + .expect("borrowing transaction_context.account_touched_flags failed"); + for (index_in_transaction, was_touched) in account_touched_flags.iter().enumerate() { + let account_data_size = context + .get_account_at_index(index_in_transaction) + .expect("index_in_transaction out of bounds") + .try_borrow() + .expect("borrowing a transaction_context.account failed") + .data() + .len() as u64; + total_size_of_all_accounts = + total_size_of_all_accounts.saturating_add(account_data_size); + if *was_touched { + changed_account_count = changed_account_count.saturating_add(1); + total_size_of_touched_accounts = + total_size_of_touched_accounts.saturating_add(account_data_size); + } + } Self { accounts: Vec::from(Pin::into_inner(context.account_keys)) .into_iter() @@ -691,7 +970,22 @@ impl From for ExecutionRecord { .collect(), instruction_trace: context.instruction_trace, return_data: context.return_data, - total_resize_delta: RefCell::into_inner(context.total_resize_delta), + changed_account_count, + total_size_of_all_accounts, + total_size_of_touched_accounts, + accounts_resize_delta: RefCell::into_inner(context.accounts_resize_delta), } } } + +fn is_zeroed(buf: &[u8]) -> bool { + const ZEROS_LEN: usize = 1024; + const ZEROS: [u8; ZEROS_LEN] = [0; ZEROS_LEN]; + let mut chunks = buf.chunks_exact(ZEROS_LEN); + + #[allow(clippy::indexing_slicing)] + { + chunks.all(|chunk| chunk == &ZEROS[..]) + && chunks.remainder() == &ZEROS[..chunks.remainder().len()] + } +}