From ef8bd0e825fed7ed3ab1255de34dfc07dd2b1412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Tue, 15 Mar 2022 00:06:52 +0100 Subject: [PATCH] Revert "Replaces KeyedAccount by BorrowedAccount in the BPF loader. (#23056)" 6c56eb9663ef0280cf7f53202425d6530d9e6ca4 --- programs/bpf_loader/src/lib.rs | 918 ++++++++++++++------------------- 1 file changed, 385 insertions(+), 533 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index b1d52b60654c38..735f56ba0e40ac 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -25,7 +25,7 @@ use { invoke_context::{ComputeMeter, Executor, InvokeContext}, log_collector::LogCollector, stable_log, - sysvar_cache::get_sysvar_with_account_check2, + sysvar_cache::get_sysvar_with_account_check, }, solana_rbpf::{ aligned_memory::AlignedMemory, @@ -37,6 +37,8 @@ use { vm::{Config, EbpfVm, InstructionMeter}, }, solana_sdk::{ + account::{ReadableAccount, WritableAccount}, + account_utils::State, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, entrypoint::{HEAP_LENGTH, SUCCESS}, @@ -46,6 +48,7 @@ use { reduce_required_deploy_balance, requestable_heap_size, }, instruction::{AccountMeta, InstructionError}, + keyed_account::{keyed_account_at_index, KeyedAccount}, loader_instruction::LoaderInstruction, loader_upgradeable_instruction::UpgradeableLoaderInstruction, program_error::MAX_ACCOUNTS_DATA_SIZE_EXCEEDED, @@ -53,7 +56,6 @@ use { pubkey::Pubkey, saturating_add_assign, system_instruction::{self, MAX_PERMITTED_DATA_LENGTH}, - transaction_context::{BorrowedAccount, InstructionContext, TransactionContext}, }, std::{cell::RefCell, fmt::Debug, pin::Pin, rc::Rc, sync::Arc}, thiserror::Error, @@ -137,15 +139,14 @@ pub fn create_executor( }; let mut create_executor_metrics = executor_metrics::CreateMetrics::default(); let mut executable = { - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let programdata = instruction_context - .try_borrow_account(transaction_context, programdata_account_index)?; - create_executor_metrics.program_id = programdata.get_key().to_string(); + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let programdata = keyed_account_at_index(keyed_accounts, programdata_account_index)?; + create_executor_metrics.program_id = programdata.unsigned_key().to_string(); let mut load_elf_time = Measure::start("load_elf_time"); let executable = Executable::::from_elf( programdata - .get_data() + .try_account_ref()? + .data() .get(programdata_offset..) .ok_or(InstructionError::AccountDataTooSmall)?, None, @@ -193,24 +194,27 @@ pub fn create_executor( })) } -fn write_program_data<'a>( - program: &mut BorrowedAccount<'a>, +fn write_program_data( + program_account_index: usize, program_data_offset: usize, bytes: &[u8], - log_collector: Option>>, + invoke_context: &mut InvokeContext, ) -> Result<(), InstructionError> { - let data = program.get_data_mut(); - let write_offset = program_data_offset.saturating_add(bytes.len()); - if data.len() < write_offset || (program_data_offset..write_offset).len() != bytes.len() { - ic_logger_msg!( - log_collector, + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let program = keyed_account_at_index(keyed_accounts, program_account_index)?; + let mut account = program.try_account_ref_mut()?; + let data = &mut account.data_as_mut_slice(); + let len = bytes.len(); + if data.len() < program_data_offset.saturating_add(len) { + ic_msg!( + invoke_context, "Write overflow: {} < {}", data.len(), - write_offset + program_data_offset.saturating_add(len), ); return Err(InstructionError::AccountDataTooSmall); } - data.get_mut(program_data_offset..write_offset) + data.get_mut(program_data_offset..program_data_offset.saturating_add(len)) .ok_or(InstructionError::AccountDataTooSmall)? .copy_from_slice(bytes); Ok(()) @@ -284,36 +288,28 @@ fn process_instruction_common( let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let program_id = instruction_context.get_program_key(transaction_context)?; - let first_account_key = transaction_context.get_key_of_account_at_index( - instruction_context.get_index_in_transaction(first_instruction_account)?, - )?; - let second_account_key = instruction_context - .get_index_in_transaction(first_instruction_account.saturating_add(1)) - .and_then(|index_in_transaction| { - transaction_context.get_key_of_account_at_index(index_in_transaction) - }); - let program_account_index = if first_account_key == program_id { - first_instruction_account - } else if second_account_key - .map(|key| key == program_id) + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let first_account = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let second_account = + keyed_account_at_index(keyed_accounts, first_instruction_account.saturating_add(1)); + let (program, next_first_instruction_account) = if first_account.unsigned_key() == program_id { + (first_account, first_instruction_account) + } else if second_account + .as_ref() + .map(|keyed_account| keyed_account.unsigned_key() == program_id) .unwrap_or(false) { - first_instruction_account.saturating_add(1) + (second_account?, first_instruction_account.saturating_add(1)) } else { - if instruction_context - .try_borrow_account(transaction_context, first_instruction_account)? - .is_executable() - { + if first_account.executable()? { ic_logger_msg!(log_collector, "BPF loader is executable"); return Err(InstructionError::IncorrectProgramId); } - first_instruction_account + (first_account, first_instruction_account) }; - let program = - instruction_context.try_borrow_account(transaction_context, program_account_index)?; - if program.is_executable() { + if program.executable()? { // First instruction account can only be zero if called from CPI, which // means stack height better be greater than one debug_assert_eq!( @@ -321,7 +317,7 @@ fn process_instruction_common( invoke_context.get_stack_height() > 1 ); - if !check_loader_id(program.get_owner()) { + if !check_loader_id(&program.owner()?) { ic_logger_msg!( log_collector, "Executable account not owned by the BPF loader" @@ -329,12 +325,12 @@ fn process_instruction_common( return Err(InstructionError::IncorrectProgramId); } - let program_data_offset = if bpf_loader_upgradeable::check_id(program.get_owner()) { + let program_data_offset = if bpf_loader_upgradeable::check_id(&program.owner()?) { if let UpgradeableLoaderState::Program { programdata_address, - } = program.get_state()? + } = program.state()? { - if programdata_address != *first_account_key { + if programdata_address != *first_account.unsigned_key() { ic_logger_msg!( log_collector, "Wrong ProgramData account for this Program account" @@ -342,9 +338,7 @@ fn process_instruction_common( return Err(InstructionError::InvalidArgument); } if !matches!( - instruction_context - .try_borrow_account(transaction_context, first_instruction_account)? - .get_state()?, + first_account.state()?, UpgradeableLoaderState::ProgramData { slot: _, upgrade_authority_address: _, @@ -361,7 +355,6 @@ fn process_instruction_common( } else { 0 }; - drop(program); let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); let executor = match invoke_context.get_executor(program_id) { @@ -387,9 +380,12 @@ fn process_instruction_common( get_or_create_executor_time.as_us() ); - executor.execute(program_account_index, instruction_data, invoke_context) + executor.execute( + next_first_instruction_account, + instruction_data, + invoke_context, + ) } else { - drop(program); debug_assert_eq!(first_instruction_account, 1); if bpf_loader_upgradeable::check_id(program_id) { process_loader_upgradeable_instruction( @@ -412,52 +408,6 @@ fn process_instruction_common( } } -pub mod upgradeable_ins_acc_idx { - pub enum InitializeBuffer { - Buffer = 0, - Authority = 1, - } - - pub enum Write { - Buffer = 0, - Authority = 1, - } - - pub enum DeployWithMaxDataLen { - Payer = 0, - ProgramData = 1, - Program = 2, - Buffer = 3, - Rent = 4, - Clock = 5, - // Ignored = 6, - UpgradeAuthority = 7, - } - - pub enum Upgrade { - ProgramData = 0, - Program = 1, - Buffer = 2, - Spill = 3, - Rent = 4, - Clock = 5, - UpgradeAuthority = 6, - } - - pub enum SetAuthority { - Account = 0, - CurrentAuthority = 1, - NewAuthority = 2, - } - - pub enum Close { - Account = 0, - Recipient = 1, - Authority = 2, - Program = 3, - } -} - fn process_loader_upgradeable_instruction( first_instruction_account: usize, instruction_data: &[u8], @@ -468,137 +418,117 @@ fn process_loader_upgradeable_instruction( let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let program_id = instruction_context.get_program_key(transaction_context)?; + let keyed_accounts = invoke_context.get_keyed_accounts()?; match limited_deserialize(instruction_data)? { UpgradeableLoaderInstruction::InitializeBuffer => { - instruction_context.check_number_of_instruction_accounts(2)?; - let mut buffer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::InitializeBuffer::Buffer as usize, - )?; - if UpgradeableLoaderState::Uninitialized != buffer.get_state()? { + let buffer = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + + if UpgradeableLoaderState::Uninitialized != buffer.state()? { ic_logger_msg!(log_collector, "Buffer account already initialized"); return Err(InstructionError::AccountAlreadyInitialized); } - let authority_address = Some(*instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::InitializeBuffer::Authority as usize, - )?); - buffer.set_state(&UpgradeableLoaderState::Buffer { authority_address })?; + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), + )?; + + buffer.set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(*authority.unsigned_key()), + })?; } UpgradeableLoaderInstruction::Write { offset, bytes } => { - instruction_context.check_number_of_instruction_accounts(2)?; - let authority_address = if let UpgradeableLoaderState::Buffer { authority_address } = - instruction_context - .try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Write::Buffer as usize, - )? - .get_state()? - { - if let Some(authority_address) = authority_address { - authority_address - } else { + let buffer = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), + )?; + + if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? { + if authority_address.is_none() { ic_logger_msg!(log_collector, "Buffer is immutable"); return Err(InstructionError::Immutable); // TODO better error code } + if authority_address != Some(*authority.unsigned_key()) { + ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if authority.signer_key().is_none() { + ic_logger_msg!(log_collector, "Buffer authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } } else { ic_logger_msg!(log_collector, "Invalid Buffer account"); return Err(InstructionError::InvalidAccountData); - }; - let current_authority_address = instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::Write::Authority as usize, - )?; - if authority_address != *current_authority_address { - ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); - return Err(InstructionError::IncorrectAuthority); - } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add(upgradeable_ins_acc_idx::Write::Authority as usize), - )? { - ic_logger_msg!(log_collector, "Buffer authority did not sign"); - return Err(InstructionError::MissingRequiredSignature); } - let mut program = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Write::Buffer as usize, - )?; write_program_data( - &mut program, + first_instruction_account, UpgradeableLoaderState::buffer_data_offset()?.saturating_add(offset as usize), &bytes, - invoke_context.get_log_collector(), + invoke_context, )?; } UpgradeableLoaderInstruction::DeployWithMaxDataLen { max_data_len } => { - instruction_context.check_number_of_instruction_accounts(4)?; - let programdata_address = *instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::ProgramData as usize, + let payer = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let programdata = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), )?; - let rent = get_sysvar_with_account_check2::rent( + let program = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), + )?; + let buffer = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(3), + )?; + let rent = get_sysvar_with_account_check::rent( + keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(4), + )?, invoke_context, - instruction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Rent as usize, )?; - let clock = get_sysvar_with_account_check2::clock( + let clock = get_sysvar_with_account_check::clock( + keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(5), + )?, invoke_context, - instruction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Clock as usize, )?; - instruction_context.check_number_of_instruction_accounts(8)?; - let upgrade_authority_address = - Some(*instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::UpgradeAuthority as usize, - )?); - let predrain_buffer = invoke_context - .feature_set - .is_active(&reduce_required_deploy_balance::id()); + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(7), + )?; + let upgrade_authority_address = Some(*authority.unsigned_key()); + let upgrade_authority_signer = authority.signer_key().is_none(); // Verify Program account - let new_program_id = { - let program = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Program as usize, - )?; - if UpgradeableLoaderState::Uninitialized != program.get_state()? { - ic_logger_msg!(log_collector, "Program account already initialized"); - return Err(InstructionError::AccountAlreadyInitialized); - } - if program.get_data().len() < UpgradeableLoaderState::program_len()? { - ic_logger_msg!(log_collector, "Program account too small"); - return Err(InstructionError::AccountDataTooSmall); - } - if program.get_lamports() < rent.minimum_balance(program.get_data().len()) { - ic_logger_msg!(log_collector, "Program account not rent-exempt"); - return Err(InstructionError::ExecutableAccountNotRentExempt); - } - *program.get_key() - }; + + if UpgradeableLoaderState::Uninitialized != program.state()? { + ic_logger_msg!(log_collector, "Program account already initialized"); + return Err(InstructionError::AccountAlreadyInitialized); + } + if program.data_len()? < UpgradeableLoaderState::program_len()? { + ic_logger_msg!(log_collector, "Program account too small"); + return Err(InstructionError::AccountDataTooSmall); + } + if program.lamports()? < rent.minimum_balance(program.data_len()?) { + ic_logger_msg!(log_collector, "Program account not rent-exempt"); + return Err(InstructionError::ExecutableAccountNotRentExempt); + } + + let new_program_id = *program.unsigned_key(); // Verify Buffer account - let mut buffer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Buffer as usize, - )?; - if let UpgradeableLoaderState::Buffer { authority_address } = buffer.get_state()? { + + if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? { if authority_address != upgrade_authority_address { ic_logger_msg!(log_collector, "Buffer and upgrade authority don't match"); return Err(InstructionError::IncorrectAuthority); } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add( - upgradeable_ins_acc_idx::DeployWithMaxDataLen::UpgradeAuthority - as usize, - ), - )? { + if upgrade_authority_signer { ic_logger_msg!(log_collector, "Upgrade authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } @@ -607,12 +537,12 @@ fn process_loader_upgradeable_instruction( return Err(InstructionError::InvalidArgument); } - let buffer_address = *buffer.get_key(); let buffer_data_offset = UpgradeableLoaderState::buffer_data_offset()?; - let buffer_data_len = buffer.get_data().len().saturating_sub(buffer_data_offset); + let buffer_data_len = buffer.data_len()?.saturating_sub(buffer_data_offset); let programdata_data_offset = UpgradeableLoaderState::programdata_data_offset()?; let programdata_len = UpgradeableLoaderState::programdata_len(max_data_len)?; - if buffer.get_data().len() < UpgradeableLoaderState::buffer_data_offset()? + + if buffer.data_len()? < UpgradeableLoaderState::buffer_data_offset()? || buffer_data_len == 0 { ic_logger_msg!(log_collector, "Buffer account too small"); @@ -634,36 +564,34 @@ fn process_loader_upgradeable_instruction( let (derived_address, bump_seed) = Pubkey::find_program_address(&[new_program_id.as_ref()], program_id); - if derived_address != programdata_address { + if derived_address != *programdata.unsigned_key() { ic_logger_msg!(log_collector, "ProgramData address is not derived"); return Err(InstructionError::InvalidArgument); } - let mut instruction = { - let mut payer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Payer as usize, - )?; - if predrain_buffer { - // Drain the Buffer account to payer before paying for programdata account - payer.checked_add_lamports(buffer.get_lamports())?; - buffer.set_lamports(0); - } + let predrain_buffer = invoke_context + .feature_set + .is_active(&reduce_required_deploy_balance::id()); + if predrain_buffer { + // Drain the Buffer account to payer before paying for programdata account + payer + .try_account_ref_mut()? + .checked_add_lamports(buffer.lamports()?)?; + buffer.try_account_ref_mut()?.set_lamports(0); + } - system_instruction::create_account( - payer.get_key(), - &programdata_address, - 1.max(rent.minimum_balance(programdata_len)), - programdata_len as u64, - program_id, - ) - }; - drop(buffer); + let mut instruction = system_instruction::create_account( + payer.unsigned_key(), + programdata.unsigned_key(), + 1.max(rent.minimum_balance(programdata_len)), + programdata_len as u64, + program_id, + ); // pass an extra account to avoid the overly strict UnbalancedInstruction error instruction .accounts - .push(AccountMeta::new(buffer_address, false)); + .push(AccountMeta::new(*buffer.unsigned_key(), false)); let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; @@ -684,138 +612,124 @@ fn process_loader_upgradeable_instruction( )?; invoke_context.update_executor(&new_program_id, executor); - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let mut buffer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Buffer as usize, + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let payer = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let programdata = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), + )?; + let program = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), + )?; + let buffer = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(3), )?; // Update the ProgramData account and record the program bits - { - let mut programdata = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::ProgramData as usize, - )?; - programdata.set_state(&UpgradeableLoaderState::ProgramData { - slot: clock.slot, - upgrade_authority_address, - })?; - - let to_slice = programdata - .get_data_mut() - .get_mut( - programdata_data_offset - ..programdata_data_offset.saturating_add(buffer_data_len), - ) - .ok_or(InstructionError::AccountDataTooSmall)?; - let from_slice = buffer - .get_data() - .get(buffer_data_offset..) - .ok_or(InstructionError::AccountDataTooSmall)?; - if to_slice.len() != from_slice.len() { - return Err(InstructionError::AccountDataTooSmall); - } - to_slice.copy_from_slice(from_slice); - } + programdata.set_state(&UpgradeableLoaderState::ProgramData { + slot: clock.slot, + upgrade_authority_address, + })?; + programdata + .try_account_ref_mut()? + .data_as_mut_slice() + .get_mut( + programdata_data_offset + ..programdata_data_offset.saturating_add(buffer_data_len), + ) + .ok_or(InstructionError::AccountDataTooSmall)? + .copy_from_slice( + buffer + .try_account_ref()? + .data() + .get(buffer_data_offset..) + .ok_or(InstructionError::AccountDataTooSmall)?, + ); // Update the Program account - { - let mut program = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Program as usize, - )?; - program.set_state(&UpgradeableLoaderState::Program { - programdata_address, - })?; - program.set_executable(true); - } + program.set_state(&UpgradeableLoaderState::Program { + programdata_address: *programdata.unsigned_key(), + })?; + program.try_account_ref_mut()?.set_executable(true); if !predrain_buffer { // Drain the Buffer account back to the payer - let mut payer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::DeployWithMaxDataLen::Payer as usize, - )?; - payer.checked_add_lamports(buffer.get_lamports())?; - buffer.set_lamports(0); + payer + .try_account_ref_mut()? + .checked_add_lamports(buffer.lamports()?)?; + buffer.try_account_ref_mut()?.set_lamports(0); } ic_logger_msg!(log_collector, "Deployed program {:?}", new_program_id); } UpgradeableLoaderInstruction::Upgrade => { - instruction_context.check_number_of_instruction_accounts(3)?; - let current_programdata_address = *instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::ProgramData as usize, + let programdata = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let program = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), )?; - let rent = get_sysvar_with_account_check2::rent( + let buffer = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), + )?; + let rent = get_sysvar_with_account_check::rent( + keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(4), + )?, invoke_context, - instruction_context, - upgradeable_ins_acc_idx::Upgrade::Rent as usize, )?; - let clock = get_sysvar_with_account_check2::clock( + let clock = get_sysvar_with_account_check::clock( + keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(5), + )?, invoke_context, - instruction_context, - upgradeable_ins_acc_idx::Upgrade::Clock as usize, )?; - instruction_context.check_number_of_instruction_accounts(7)?; - let current_upgrade_authority_address = - Some(*instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::UpgradeAuthority as usize, - )?); + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(6), + )?; // Verify Program account - let new_program_id = { - let program = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::Program as usize, - )?; - if !program.is_executable() { - ic_logger_msg!(log_collector, "Program account not executable"); - return Err(InstructionError::AccountNotExecutable); - } - if !program.is_writable() { - ic_logger_msg!(log_collector, "Program account not writeable"); + + if !program.executable()? { + ic_logger_msg!(log_collector, "Program account not executable"); + return Err(InstructionError::AccountNotExecutable); + } + if !program.is_writable() { + ic_logger_msg!(log_collector, "Program account not writeable"); + return Err(InstructionError::InvalidArgument); + } + if &program.owner()? != program_id { + ic_logger_msg!(log_collector, "Program account not owned by loader"); + return Err(InstructionError::IncorrectProgramId); + } + if let UpgradeableLoaderState::Program { + programdata_address, + } = program.state()? + { + if programdata_address != *programdata.unsigned_key() { + ic_logger_msg!(log_collector, "Program and ProgramData account mismatch"); return Err(InstructionError::InvalidArgument); } - if program.get_owner() != program_id { - ic_logger_msg!(log_collector, "Program account not owned by loader"); - return Err(InstructionError::IncorrectProgramId); - } - if let UpgradeableLoaderState::Program { - programdata_address, - } = program.get_state()? - { - if current_programdata_address != programdata_address { - ic_logger_msg!(log_collector, "Program and ProgramData account mismatch"); - return Err(InstructionError::InvalidArgument); - } - } else { - ic_logger_msg!(log_collector, "Invalid Program account"); - return Err(InstructionError::InvalidAccountData); - } - *program.get_key() - }; + } else { + ic_logger_msg!(log_collector, "Invalid Program account"); + return Err(InstructionError::InvalidAccountData); + } + + let new_program_id = *program.unsigned_key(); // Verify Buffer account - let buffer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::Buffer as usize, - )?; - if let UpgradeableLoaderState::Buffer { authority_address } = buffer.get_state()? { - if authority_address != current_upgrade_authority_address { + + if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? { + if authority_address != Some(*authority.unsigned_key()) { ic_logger_msg!(log_collector, "Buffer and upgrade authority don't match"); return Err(InstructionError::IncorrectAuthority); } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add( - upgradeable_ins_acc_idx::Upgrade::UpgradeAuthority as usize, - ), - )? { + if authority.signer_key().is_none() { ic_logger_msg!(log_collector, "Upgrade authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } @@ -825,15 +739,11 @@ fn process_loader_upgradeable_instruction( } let buffer_data_offset = UpgradeableLoaderState::buffer_data_offset()?; - let buffer_data_len = buffer.get_data().len().saturating_sub(buffer_data_offset); - let programdata = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::ProgramData as usize, - )?; + let buffer_data_len = buffer.data_len()?.saturating_sub(buffer_data_offset); let programdata_data_offset = UpgradeableLoaderState::programdata_data_offset()?; - let programdata_balance_required = - 1.max(rent.minimum_balance(programdata.get_data().len())); - if buffer.get_data().len() < UpgradeableLoaderState::buffer_data_offset()? + let programdata_balance_required = 1.max(rent.minimum_balance(programdata.data_len()?)); + + if buffer.data_len()? < UpgradeableLoaderState::buffer_data_offset()? || buffer_data_len == 0 { ic_logger_msg!(log_collector, "Buffer account too small"); @@ -841,15 +751,12 @@ fn process_loader_upgradeable_instruction( } // Verify ProgramData account - if programdata.get_data().len() - < UpgradeableLoaderState::programdata_len(buffer_data_len)? - { + + if programdata.data_len()? < UpgradeableLoaderState::programdata_len(buffer_data_len)? { ic_logger_msg!(log_collector, "ProgramData account not large enough"); return Err(InstructionError::AccountDataTooSmall); } - if programdata - .get_lamports() - .saturating_add(buffer.get_lamports()) + if programdata.lamports()?.saturating_add(buffer.lamports()?) < programdata_balance_required { ic_logger_msg!( @@ -858,27 +765,20 @@ fn process_loader_upgradeable_instruction( ); return Err(InstructionError::InsufficientFunds); } - drop(buffer); if let UpgradeableLoaderState::ProgramData { slot: _, upgrade_authority_address, - } = programdata.get_state()? + } = programdata.state()? { if upgrade_authority_address.is_none() { ic_logger_msg!(log_collector, "Program not upgradeable"); return Err(InstructionError::Immutable); } - if upgrade_authority_address != current_upgrade_authority_address { + if upgrade_authority_address != Some(*authority.unsigned_key()) { ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); return Err(InstructionError::IncorrectAuthority); } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add( - upgradeable_ins_acc_idx::Upgrade::UpgradeAuthority as usize, - ), - )? { + if authority.signer_key().is_none() { ic_logger_msg!(log_collector, "Upgrade authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } @@ -886,7 +786,6 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Invalid ProgramData account"); return Err(InstructionError::InvalidAccountData); } - drop(programdata); // Load and verify the program bits let executor = create_executor( @@ -898,80 +797,76 @@ fn process_loader_upgradeable_instruction( )?; invoke_context.update_executor(&new_program_id, executor); - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let mut programdata = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::ProgramData as usize, + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let programdata = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let buffer = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), )?; - let mut buffer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::Buffer as usize, + let spill = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(3), + )?; + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(6), )?; - // Update the ProgramData account, record the upgraded data, and zero the rest + // Update the ProgramData account, record the upgraded data, and zero + // the rest programdata.set_state(&UpgradeableLoaderState::ProgramData { slot: clock.slot, - upgrade_authority_address: current_upgrade_authority_address, + upgrade_authority_address: Some(*authority.unsigned_key()), })?; - let to_slice = programdata - .get_data_mut() + programdata + .try_account_ref_mut()? + .data_as_mut_slice() .get_mut( programdata_data_offset ..programdata_data_offset.saturating_add(buffer_data_len), ) - .ok_or(InstructionError::AccountDataTooSmall)?; - let from_slice = buffer - .get_data() - .get(buffer_data_offset..) - .ok_or(InstructionError::AccountDataTooSmall)?; - if to_slice.len() != from_slice.len() { - return Err(InstructionError::AccountDataTooSmall); - } - to_slice.copy_from_slice(from_slice); + .ok_or(InstructionError::AccountDataTooSmall)? + .copy_from_slice( + buffer + .try_account_ref()? + .data() + .get(buffer_data_offset..) + .ok_or(InstructionError::AccountDataTooSmall)?, + ); programdata - .get_data_mut() + .try_account_ref_mut()? + .data_as_mut_slice() .get_mut(programdata_data_offset.saturating_add(buffer_data_len)..) .ok_or(InstructionError::AccountDataTooSmall)? .fill(0); // Fund ProgramData to rent-exemption, spill the rest - { - let mut spill = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::Spill as usize, - )?; - spill.checked_add_lamports( - (programdata - .get_lamports() - .saturating_add(buffer.get_lamports())) + + spill.try_account_ref_mut()?.checked_add_lamports( + programdata + .lamports()? + .saturating_add(buffer.lamports()?) .saturating_sub(programdata_balance_required), - )?; - buffer.set_lamports(0); - programdata.set_lamports(programdata_balance_required); - } + )?; + buffer.try_account_ref_mut()?.set_lamports(0); + programdata + .try_account_ref_mut()? + .set_lamports(programdata_balance_required); ic_logger_msg!(log_collector, "Upgraded program {:?}", new_program_id); } UpgradeableLoaderInstruction::SetAuthority => { - instruction_context.check_number_of_instruction_accounts(2)?; - let mut account = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::SetAuthority::Account as usize, + let account = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let present_authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), )?; - let present_authority = Some(*instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::SetAuthority::CurrentAuthority as usize, - )?); - let new_authority = instruction_context - .get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::SetAuthority::NewAuthority as usize, - ) - .ok() - .cloned(); + let new_authority = + keyed_account_at_index(keyed_accounts, first_instruction_account.saturating_add(2)) + .ok() + .map(|account| account.unsigned_key()); - match account.get_state()? { + match account.state()? { UpgradeableLoaderState::Buffer { authority_address } => { if new_authority.is_none() { ic_logger_msg!(log_collector, "Buffer authority is not optional"); @@ -981,22 +876,16 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Buffer is immutable"); return Err(InstructionError::Immutable); } - if authority_address != present_authority { + if authority_address != Some(*present_authority.unsigned_key()) { ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); return Err(InstructionError::IncorrectAuthority); } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add( - upgradeable_ins_acc_idx::SetAuthority::CurrentAuthority as usize, - ), - )? { + if present_authority.signer_key().is_none() { ic_logger_msg!(log_collector, "Buffer authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } account.set_state(&UpgradeableLoaderState::Buffer { - authority_address: new_authority, + authority_address: new_authority.cloned(), })?; } UpgradeableLoaderState::ProgramData { @@ -1007,23 +896,17 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Program not upgradeable"); return Err(InstructionError::Immutable); } - if upgrade_authority_address != present_authority { + if upgrade_authority_address != Some(*present_authority.unsigned_key()) { ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); return Err(InstructionError::IncorrectAuthority); } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add( - upgradeable_ins_acc_idx::SetAuthority::CurrentAuthority as usize, - ), - )? { + if present_authority.signer_key().is_none() { ic_logger_msg!(log_collector, "Upgrade authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } account.set_state(&UpgradeableLoaderState::ProgramData { slot, - upgrade_authority_address: new_authority, + upgrade_authority_address: new_authority.cloned(), })?; } _ => { @@ -1035,82 +918,75 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "New authority {:?}", new_authority); } UpgradeableLoaderInstruction::Close => { - instruction_context.check_number_of_instruction_accounts(2)?; - if instruction_context.get_index_in_transaction( - first_instruction_account - .saturating_add(upgradeable_ins_acc_idx::Close::Account as usize), - )? == instruction_context.get_index_in_transaction( - first_instruction_account - .saturating_add(upgradeable_ins_acc_idx::Close::Recipient as usize), - )? { + let close_account = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let recipient_account = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(1), + )?; + if close_account.unsigned_key() == recipient_account.unsigned_key() { ic_logger_msg!( log_collector, "Recipient is the same as the account being closed" ); return Err(InstructionError::InvalidArgument); } - let mut close_account = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Close::Account as usize, - )?; - match close_account.get_state()? { + match close_account.state()? { UpgradeableLoaderState::Uninitialized => { - let mut recipient_account = instruction_context - .try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Close::Recipient as usize, - )?; - recipient_account.checked_add_lamports(close_account.get_lamports())?; - close_account.set_lamports(0); + recipient_account + .try_account_ref_mut()? + .checked_add_lamports(close_account.lamports()?)?; + close_account.try_account_ref_mut()?.set_lamports(0); ic_logger_msg!( log_collector, "Closed Uninitialized {}", - close_account.get_key() + close_account.unsigned_key() ); } UpgradeableLoaderState::Buffer { authority_address } => { - instruction_context.check_number_of_instruction_accounts(3)?; - drop(close_account); + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), + )?; + common_close_account( &authority_address, - transaction_context, - instruction_context, + authority, + close_account, + recipient_account, &log_collector, )?; - let close_account = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Close::Account as usize, - )?; - ic_logger_msg!(log_collector, "Closed Buffer {}", close_account.get_key()); + ic_logger_msg!( + log_collector, + "Closed Buffer {}", + close_account.unsigned_key() + ); } UpgradeableLoaderState::ProgramData { slot: _, upgrade_authority_address: authority_address, } => { - instruction_context.check_number_of_instruction_accounts(4)?; - if !instruction_context.is_writable( - instruction_context - .get_number_of_program_accounts() - .saturating_add(upgradeable_ins_acc_idx::Close::Program as usize), - )? { + let program_account = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(3), + )?; + + if !program_account.is_writable() { ic_logger_msg!(log_collector, "Program account is not writable"); return Err(InstructionError::InvalidArgument); } - let program_account = instruction_context - .try_borrow_instruction_account(transaction_context, 3)?; - if program_account.get_owner() != program_id { + if &program_account.owner()? != program_id { ic_logger_msg!(log_collector, "Program account not owned by loader"); return Err(InstructionError::IncorrectProgramId); } - match program_account.get_state()? { + match program_account.state()? { UpgradeableLoaderState::Program { programdata_address, } => { - if programdata_address != *close_account.get_key() { + if programdata_address != *close_account.unsigned_key() { ic_logger_msg!( log_collector, "ProgramData account does not match ProgramData account" @@ -1118,11 +994,15 @@ fn process_loader_upgradeable_instruction( return Err(InstructionError::InvalidArgument); } - drop(close_account); + let authority = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), + )?; common_close_account( &authority_address, - transaction_context, - instruction_context, + authority, + close_account, + recipient_account, &log_collector, )?; } @@ -1135,7 +1015,7 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!( log_collector, "Closed Program {}", - program_account.get_key() + program_account.unsigned_key() ); } _ => { @@ -1151,55 +1031,32 @@ fn process_loader_upgradeable_instruction( fn common_close_account( authority_address: &Option, - transaction_context: &TransactionContext, - instruction_context: &InstructionContext, + authority_account: &KeyedAccount, + close_account: &KeyedAccount, + recipient_account: &KeyedAccount, log_collector: &Option>>, ) -> Result<(), InstructionError> { if authority_address.is_none() { ic_logger_msg!(log_collector, "Account is immutable"); return Err(InstructionError::Immutable); } - if *authority_address - != Some(*instruction_context.get_instruction_account_key( - transaction_context, - upgradeable_ins_acc_idx::Close::Authority as usize, - )?) - { + if *authority_address != Some(*authority_account.unsigned_key()) { ic_logger_msg!(log_collector, "Incorrect authority provided"); return Err(InstructionError::IncorrectAuthority); } - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add(upgradeable_ins_acc_idx::Close::Authority as usize), - )? { + if authority_account.signer_key().is_none() { ic_logger_msg!(log_collector, "Authority did not sign"); return Err(InstructionError::MissingRequiredSignature); } - let mut close_account = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Close::Account as usize, - )?; - let mut recipient_account = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Close::Recipient as usize, - )?; - recipient_account.checked_add_lamports(close_account.get_lamports())?; - close_account.set_lamports(0); + + recipient_account + .try_account_ref_mut()? + .checked_add_lamports(close_account.lamports()?)?; + close_account.try_account_ref_mut()?.set_lamports(0); close_account.set_state(&UpgradeableLoaderState::Uninitialized)?; Ok(()) } -pub mod deprecated_ins_acc_idx { - pub enum Write { - Program = 0, - } - - pub enum Finalize { - Program = 0, - } -} - fn process_loader_instruction( first_instruction_account: usize, instruction_data: &[u8], @@ -1209,11 +1066,9 @@ fn process_loader_instruction( let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let program_id = instruction_context.get_program_key(transaction_context)?; - if instruction_context - .try_borrow_instruction_account(transaction_context, 0)? - .get_owner() - != program_id - { + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let program = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + if program.owner()? != *program_id { ic_msg!( invoke_context, "Executable account not owned by the BPF loader" @@ -1222,44 +1077,34 @@ fn process_loader_instruction( } match limited_deserialize(instruction_data)? { LoaderInstruction::Write { offset, bytes } => { - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add(deprecated_ins_acc_idx::Write::Program as usize), - )? { + if program.signer_key().is_none() { ic_msg!(invoke_context, "Program account did not sign"); return Err(InstructionError::MissingRequiredSignature); } write_program_data( - &mut instruction_context.try_borrow_instruction_account( - transaction_context, - deprecated_ins_acc_idx::Write::Program as usize, - )?, + first_instruction_account, offset as usize, &bytes, - invoke_context.get_log_collector(), + invoke_context, )?; } LoaderInstruction::Finalize => { - if !instruction_context.is_signer( - instruction_context - .get_number_of_program_accounts() - .saturating_add(deprecated_ins_acc_idx::Finalize::Program as usize), - )? { - ic_msg!(invoke_context, "Program account did not sign"); + if program.signer_key().is_none() { + ic_msg!(invoke_context, "key[0] did not sign the transaction"); return Err(InstructionError::MissingRequiredSignature); } + let executor = create_executor(first_instruction_account, 0, invoke_context, use_jit, true)?; - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let mut program = instruction_context.try_borrow_instruction_account( - transaction_context, - deprecated_ins_acc_idx::Finalize::Program as usize, - )?; - invoke_context.update_executor(program.get_key(), executor); - program.set_executable(true); - ic_msg!(invoke_context, "Finalized account {:?}", program.get_key()); + let keyed_accounts = invoke_context.get_keyed_accounts()?; + let program = keyed_account_at_index(keyed_accounts, first_instruction_account)?; + invoke_context.update_executor(program.unsigned_key(), executor); + program.try_account_ref_mut()?.set_executable(true); + ic_msg!( + invoke_context, + "Finalized account {:?}", + program.unsigned_key() + ); } } @@ -1441,7 +1286,6 @@ mod tests { solana_sdk::{ account::{ create_account_shared_data_for_test as create_account_for_test, AccountSharedData, - ReadableAccount, WritableAccount, }, account_utils::StateMut, client::SyncClient, @@ -2241,12 +2085,13 @@ mod tests { let fee_calculator = genesis_config.fee_rate_governor.create_fee_calculator(); 3 * fee_calculator.lamports_per_signature }; - let min_payer_balance = - min_program_balance + min_programdata_balance - min_buffer_balance + deploy_fees; + let min_payer_balance = min_program_balance + .saturating_add(min_programdata_balance) + .saturating_sub(min_buffer_balance.saturating_add(deploy_fees)); bank.store_account( &payer_keypair.pubkey(), &AccountSharedData::new( - payer_base_balance + min_payer_balance, + payer_base_balance.saturating_add(min_payer_balance), 0, &system_program::id(), ), @@ -2488,7 +2333,7 @@ mod tests { &program_keypair.pubkey(), &buffer_address, &upgrade_authority_keypair.pubkey(), - min_program_balance - 1, + min_program_balance.saturating_sub(1), elf.len(), ) .unwrap(), @@ -2523,7 +2368,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), min_program_balance, - UpgradeableLoaderState::program_len().unwrap() as u64 + 1, + (UpgradeableLoaderState::program_len().unwrap() as u64).saturating_add(1), &id(), ); let message = Message::new(&instructions, Some(&mint_keypair.pubkey())); @@ -2556,7 +2401,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), min_program_balance, - UpgradeableLoaderState::program_len().unwrap() as u64 - 1, + (UpgradeableLoaderState::program_len().unwrap() as u64).saturating_sub(1), &id(), ); let message = Message::new(&instructions, Some(&mint_keypair.pubkey())); @@ -2576,7 +2421,11 @@ mod tests { bank.clear_signatures(); bank.store_account( &mint_keypair.pubkey(), - &AccountSharedData::new(deploy_fees + min_program_balance, 0, &system_program::id()), + &AccountSharedData::new( + deploy_fees.saturating_add(min_program_balance), + 0, + &system_program::id(), + ), ); bank.store_account(&buffer_address, &buffer_account); bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default()); @@ -2620,7 +2469,7 @@ mod tests { &buffer_address, &upgrade_authority_keypair.pubkey(), min_program_balance, - elf.len() - 1, + elf.len().saturating_sub(1), ) .unwrap(), Some(&mint_keypair.pubkey()), @@ -3040,7 +2889,9 @@ mod tests { .data() .get( UpgradeableLoaderState::programdata_data_offset().unwrap() - ..UpgradeableLoaderState::programdata_data_offset().unwrap() + elf_new.len(), + ..UpgradeableLoaderState::programdata_data_offset() + .unwrap() + .saturating_add(elf_new.len()), ) .unwrap() .iter() @@ -3224,7 +3075,8 @@ mod tests { ); transaction_accounts.get_mut(2).unwrap().1 = AccountSharedData::new( 1, - UpgradeableLoaderState::buffer_len(elf_orig.len().max(elf_new.len()) + 1).unwrap(), + UpgradeableLoaderState::buffer_len(elf_orig.len().max(elf_new.len()).saturating_add(1)) + .unwrap(), &bpf_loader_upgradeable::id(), ); transaction_accounts