From 905b2d1d2a211b2bc60958efa0293c857e693d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 14 Mar 2022 14:58:36 +0100 Subject: [PATCH] Revert "Replaces KeyedAccount by BorrowedAccount in the BPF loader. (#23056)" 6c56eb9663ef0280cf7f53202425d6530d9e6ca4 --- programs/bpf_loader/src/lib.rs | 914 ++++++++++++++------------------- 1 file changed, 381 insertions(+), 533 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 7b3bd371a23896..2d585b4255d26f 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, @@ -190,24 +191,27 @@ pub fn create_executor( Ok(Arc::new(BpfExecutor { executable })) } -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(()) @@ -281,36 +285,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!( @@ -318,7 +314,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" @@ -326,12 +322,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" @@ -339,9 +335,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: _, @@ -358,7 +352,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) { @@ -385,13 +378,12 @@ fn process_instruction_common( ); executor.execute( - program_account_index, + next_first_instruction_account, instruction_data, invoke_context, use_jit, ) } else { - drop(program); debug_assert_eq!(first_instruction_account, 1); if bpf_loader_upgradeable::check_id(program_id) { process_loader_upgradeable_instruction( @@ -414,52 +406,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], @@ -470,137 +416,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 program = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), )?; - let rent = get_sysvar_with_account_check2::rent( + 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); } @@ -609,12 +535,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"); @@ -636,36 +562,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()?; @@ -686,138 +610,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 buffer = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(2), )?; - let rent = get_sysvar_with_account_check2::rent( + 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); } @@ -827,15 +737,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"); @@ -843,15 +749,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!( @@ -860,27 +763,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); } @@ -888,7 +784,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( @@ -900,80 +795,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 spill = keyed_account_at_index( + keyed_accounts, + first_instruction_account.saturating_add(3), )?; - let mut buffer = instruction_context.try_borrow_instruction_account( - transaction_context, - upgradeable_ins_acc_idx::Upgrade::Buffer as usize, + 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"); @@ -983,22 +874,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 { @@ -1009,23 +894,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(), })?; } _ => { @@ -1037,82 +916,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" @@ -1120,11 +992,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, )?; } @@ -1137,7 +1013,7 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!( log_collector, "Closed Program {}", - program_account.get_key() + program_account.unsigned_key() ); } _ => { @@ -1153,55 +1029,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], @@ -1211,11 +1064,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" @@ -1224,44 +1075,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() + ); } } @@ -1443,7 +1284,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, @@ -2243,12 +2083,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(), ), @@ -2490,7 +2331,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(), @@ -2525,7 +2366,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())); @@ -2558,7 +2399,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())); @@ -2578,7 +2419,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()); @@ -2622,7 +2467,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()), @@ -3042,7 +2887,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() @@ -3226,7 +3073,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