From 9674b78f3a221df0f3a41343911dff65e197ade5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 24 Mar 2023 20:05:52 +0100 Subject: [PATCH 1/8] Moves stable_log::program_invoke(), stable_log::program_success() and stable_log::program_failure() calls from bpf_loader into InvokeContext::process_executable_chain(). --- program-runtime/src/invoke_context.rs | 27 +++++++++++---------------- programs/bpf_loader/src/lib.rs | 8 -------- programs/sbf/tests/programs.rs | 10 ++++------ 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 86a3cdc511533c..a6e6465c6d3c12 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -689,26 +689,21 @@ impl<'a> InvokeContext<'a> { self.transaction_context .set_return_data(program_id, Vec::new())?; - let is_builtin_program = builtin_id == program_id; let pre_remaining_units = self.get_remaining(); - let result = if is_builtin_program { - let logger = self.get_log_collector(); - stable_log::program_invoke(&logger, &program_id, self.get_stack_height()); - (entry.process_instruction)(self) - .map(|()| { - stable_log::program_success(&logger, &program_id); - }) - .map_err(|err| { - stable_log::program_failure(&logger, &program_id, &err); - err - }) - } else { - (entry.process_instruction)(self) - }; + let logger = self.get_log_collector(); + stable_log::program_invoke(&logger, &program_id, self.get_stack_height()); + let result = (entry.process_instruction)(self) + .map(|()| { + stable_log::program_success(&logger, &program_id); + }) + .map_err(|err| { + stable_log::program_failure(&logger, &program_id, &err); + err + }); let post_remaining_units = self.get_remaining(); *compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); - if is_builtin_program + if builtin_id == program_id && result.is_ok() && *compute_units_consumed == 0 && self diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index a2a4fb465ea39f..75b7181a7529e5 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1527,7 +1527,6 @@ fn execute<'a, 'b: 'a>( invoke_context: &'a mut InvokeContext<'b>, ) -> Result<(), InstructionError> { let log_collector = invoke_context.get_log_collector(); - let stack_height = invoke_context.get_stack_height(); let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let program_id = *instruction_context.get_last_program_key(transaction_context)?; @@ -1575,7 +1574,6 @@ fn execute<'a, 'b: 'a>( create_vm_time.stop(); execute_time = Measure::start("execute"); - stable_log::program_invoke(&log_collector, &program_id, stack_height); let (compute_units_consumed, result) = vm.execute_program(!use_jit); drop(vm); ic_logger_msg!( @@ -1609,7 +1607,6 @@ fn execute<'a, 'b: 'a>( } else { status.into() }; - stable_log::program_failure(&log_collector, &program_id, &error); Err(error) } ProgramResult::Err(error) => { @@ -1631,11 +1628,9 @@ fn execute<'a, 'b: 'a>( } } err => { - ic_logger_msg!(log_collector, "Program failed to complete: {}", err); InstructionError::ProgramFailedToComplete } }; - stable_log::program_failure(&log_collector, &program_id, &error); Err(error) } _ => Ok(()), @@ -1665,9 +1660,6 @@ fn execute<'a, 'b: 'a>( .deserialize_us .saturating_add(deserialize_time.as_us()); - if execute_or_deserialize_result.is_ok() { - stable_log::program_success(&log_collector, &program_id); - } execute_or_deserialize_result } diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 007cbde7bb2b84..a0604789b1afd0 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -918,6 +918,7 @@ fn test_program_sbf_invoke_sanity() { assert_eq!(result, Err(expected_error)); assert_eq!(invoked_programs, expected_invoked_programs); if let Some(expected_log_messages) = expected_log_messages { + assert_eq!(log_messages.len(), expected_log_messages.len()); expected_log_messages .into_iter() .zip(log_messages) @@ -985,8 +986,7 @@ fn test_program_sbf_invoke_sanity() { format!("Program log: invoke {program_lang} program"), "Program log: Test max instruction data len exceeded".into(), "skip".into(), // don't compare compute consumption logs - "Program failed to complete: Invoked an instruction with data that is too large (10241 > 10240)".into(), - format!("Program {invoke_program_id} failed: Program failed to complete"), + format!("Program {invoke_program_id} failed: Invoked an instruction with data that is too large (10241 > 10240)"), ]), ); @@ -999,8 +999,7 @@ fn test_program_sbf_invoke_sanity() { format!("Program log: invoke {program_lang} program"), "Program log: Test max instruction accounts exceeded".into(), "skip".into(), // don't compare compute consumption logs - "Program failed to complete: Invoked an instruction with too many accounts (256 > 255)".into(), - format!("Program {invoke_program_id} failed: Program failed to complete"), + format!("Program {invoke_program_id} failed: Invoked an instruction with too many accounts (256 > 255)"), ]), ); @@ -1013,8 +1012,7 @@ fn test_program_sbf_invoke_sanity() { format!("Program log: invoke {program_lang} program"), "Program log: Test max account infos exceeded".into(), "skip".into(), // don't compare compute consumption logs - "Program failed to complete: Invoked an instruction with too many account info's (129 > 128)".into(), - format!("Program {invoke_program_id} failed: Program failed to complete"), + format!("Program {invoke_program_id} failed: Invoked an instruction with too many account info's (129 > 128)"), ]), ); From b3cae935fe50924c14a2feb2e57684d1d984531d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Fri, 24 Mar 2023 18:52:18 +0100 Subject: [PATCH 2/8] Turns result of ProcessInstructionWithContext from InstructionError into Box. --- ledger/src/blockstore_processor.rs | 14 +++--- program-runtime/src/invoke_context.rs | 49 ++++++++++++++----- program-runtime/src/stable_log.rs | 4 +- program-test/src/lib.rs | 6 +-- .../address-lookup-table/src/processor.rs | 14 ++---- programs/bpf_loader/src/lib.rs | 20 +++++--- programs/compute-budget/src/lib.rs | 9 ++-- programs/config/src/config_processor.rs | 15 ++---- programs/loader-v3/src/lib.rs | 25 ++++++---- programs/stake/src/stake_instruction.rs | 16 +++--- programs/vote/src/vote_processor.rs | 20 +++----- programs/zk-token-proof/src/lib.rs | 31 +++++++++--- runtime/benches/bank.rs | 6 +-- runtime/src/bank/tests.rs | 48 +++++++++--------- runtime/src/message_processor.rs | 16 +++--- runtime/src/system_instruction_processor.rs | 16 +++--- 16 files changed, 169 insertions(+), 140 deletions(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 43fbbc37ee0f1b..119d0b7fc92420 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -2975,7 +2975,7 @@ pub mod tests { fn mock_processor_ok( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { invoke_context.consume_checked(1)?; Ok(()) } @@ -3006,7 +3006,7 @@ pub mod tests { fn mock_processor_err( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { let instruction_errors = get_instruction_errors(); invoke_context.consume_checked(1)?; @@ -3017,10 +3017,12 @@ pub mod tests { .get_instruction_data() .first() .expect("Failed to get instruction data"); - Err(instruction_errors - .get(*err as usize) - .expect("Invalid error index") - .clone()) + Err(Box::new( + instruction_errors + .get(*err as usize) + .expect("Invalid error index") + .clone(), + )) } let mut bankhash_err = None; diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index a6e6465c6d3c12..7a3b7d43fce823 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -38,7 +38,27 @@ use { }, }; -pub type ProcessInstructionWithContext = fn(&mut InvokeContext) -> Result<(), InstructionError>; +/// Adapter so we can unify the interfaces of built-in programs and syscalls +#[macro_export] +macro_rules! declare_process_instruction { + ($cu_to_consume:expr) => { + pub fn process_instruction( + invoke_context: &mut InvokeContext, + ) -> Result<(), Box> { + if invoke_context + .feature_set + .is_active(&feature_set::native_programs_consume_cu::id()) + { + invoke_context.consume_checked($cu_to_consume)?; + } + process_instruction_inner(invoke_context) + .map_err(|err| Box::new(err) as Box) + } + }; +} + +pub type ProcessInstructionWithContext = + fn(&mut InvokeContext) -> Result<(), Box>; #[derive(Clone)] pub struct BuiltinProgram { @@ -50,7 +70,7 @@ impl std::fmt::Debug for BuiltinProgram { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { // These are just type aliases for work around of Debug-ing above pointers type ErasedProcessInstructionWithContext = - fn(&'static mut InvokeContext<'static>) -> Result<(), InstructionError>; + fn(&'static mut InvokeContext<'static>) -> Result<(), Box>; // rustc doesn't compile due to bug without this work around // https://github.com/rust-lang/rust/issues/50280 @@ -697,8 +717,12 @@ impl<'a> InvokeContext<'a> { stable_log::program_success(&logger, &program_id); }) .map_err(|err| { - stable_log::program_failure(&logger, &program_id, &err); - err + stable_log::program_failure(&logger, &program_id, err.as_ref()); + if let Some(err) = err.downcast_ref::() { + err.clone() + } else { + InstructionError::ProgramFailedToComplete + } }); let post_remaining_units = self.get_remaining(); *compute_units_consumed = pre_remaining_units.saturating_sub(post_remaining_units); @@ -981,14 +1005,14 @@ mod tests { #[test] fn test_program_entry_debug() { - #[allow(clippy::unnecessary_wraps)] fn mock_process_instruction( _invoke_context: &mut InvokeContext, - ) -> Result<(), InstructionError> { + ) -> Result<(), Box> { Ok(()) } - #[allow(clippy::unnecessary_wraps)] - fn mock_ix_processor(_invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { + fn mock_ix_processor( + _invoke_context: &mut InvokeContext, + ) -> Result<(), Box> { Ok(()) } let builtin_programs = &[ @@ -1009,7 +1033,7 @@ mod tests { #[allow(clippy::integer_arithmetic)] fn mock_process_instruction( invoke_context: &mut InvokeContext, - ) -> Result<(), InstructionError> { + ) -> Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -1043,7 +1067,7 @@ mod tests { if let Ok(instruction) = bincode::deserialize(instruction_data) { match instruction { MockInstruction::NoopSuccess => (), - MockInstruction::NoopFail => return Err(InstructionError::GenericError), + MockInstruction::NoopFail => return Err(Box::new(InstructionError::GenericError)), MockInstruction::ModifyOwned => instruction_context .try_borrow_instruction_account(transaction_context, 0)? .set_data_from_slice(&[1])?, @@ -1093,14 +1117,15 @@ mod tests { desired_result, } => { invoke_context.consume_checked(compute_units_to_consume)?; - return desired_result; + return desired_result + .map_err(|err| Box::new(err) as Box); } MockInstruction::Resize { new_len } => instruction_context .try_borrow_instruction_account(transaction_context, 0)? .set_data(vec![0; new_len as usize])?, } } else { - return Err(InstructionError::InvalidInstructionData); + return Err(Box::new(InstructionError::InvalidInstructionData)); } Ok(()) } diff --git a/program-runtime/src/stable_log.rs b/program-runtime/src/stable_log.rs index 4806562f29dd7b..7633e640dfbcbe 100644 --- a/program-runtime/src/stable_log.rs +++ b/program-runtime/src/stable_log.rs @@ -5,7 +5,7 @@ use { crate::{ic_logger_msg, log_collector::LogCollector}, itertools::Itertools, - solana_sdk::{instruction::InstructionError, pubkey::Pubkey}, + solana_sdk::pubkey::Pubkey, std::{cell::RefCell, rc::Rc}, }; @@ -103,7 +103,7 @@ pub fn program_success(log_collector: &Option>>, progra pub fn program_failure( log_collector: &Option>>, program_id: &Pubkey, - err: &InstructionError, + err: &dyn std::error::Error, ) { ic_logger_msg!(log_collector, "Program {} failed: {}", program_id, err); } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 6062a9e1d6e46a..3d41533aba6920 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -100,7 +100,7 @@ fn get_invoke_context<'a, 'b>() -> &'a mut InvokeContext<'b> { pub fn builtin_process_instruction( process_instruction: solana_sdk::entrypoint::ProcessInstruction, invoke_context: &mut InvokeContext, -) -> Result<(), InstructionError> { +) -> Result<(), Box> { set_invoke_context(invoke_context); let transaction_context = &invoke_context.transaction_context; @@ -134,8 +134,8 @@ pub fn builtin_process_instruction( // Execute the program process_instruction(program_id, &account_infos, instruction_data).map_err(|err| { - let err = u64::from(err); - stable_log::program_failure(&log_collector, program_id, &err.into()); + let err: Box = Box::new(InstructionError::from(u64::from(err))); + stable_log::program_failure(&log_collector, program_id, err.as_ref()); err })?; stable_log::program_success(&log_collector, program_id); diff --git a/programs/address-lookup-table/src/processor.rs b/programs/address-lookup-table/src/processor.rs index 06891725f41258..5103398d96a06a 100644 --- a/programs/address-lookup-table/src/processor.rs +++ b/programs/address-lookup-table/src/processor.rs @@ -6,7 +6,7 @@ use { LOOKUP_TABLE_MAX_ADDRESSES, LOOKUP_TABLE_META_SIZE, }, }, - solana_program_runtime::{ic_msg, invoke_context::InvokeContext}, + solana_program_runtime::{declare_process_instruction, ic_msg, invoke_context::InvokeContext}, solana_sdk::{ clock::Slot, feature_set, @@ -18,14 +18,10 @@ use { std::convert::TryFrom, }; -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { - // Consume compute units if feature `native_programs_consume_cu` is activated, - if invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()) - { - invoke_context.consume_checked(750)?; - } +declare_process_instruction!(750); +pub fn process_instruction_inner( + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 75b7181a7529e5..f0624f3eb9645b 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -432,15 +432,21 @@ fn create_memory_mapping<'a, 'b, C: ContextObject>( }) } -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { - process_instruction_common(invoke_context, false) +pub fn process_instruction( + invoke_context: &mut InvokeContext, +) -> Result<(), Box> { + process_instruction_inner(invoke_context, false) + .map_err(|err| Box::new(err) as Box) } -pub fn process_instruction_jit(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { - process_instruction_common(invoke_context, true) +pub fn process_instruction_jit( + invoke_context: &mut InvokeContext, +) -> Result<(), Box> { + process_instruction_inner(invoke_context, true) + .map_err(|err| Box::new(err) as Box) } -fn process_instruction_common( +fn process_instruction_inner( invoke_context: &mut InvokeContext, use_jit: bool, ) -> Result<(), InstructionError> { @@ -1627,9 +1633,7 @@ fn execute<'a, 'b: 'a>( _ => unreachable!(), } } - err => { - InstructionError::ProgramFailedToComplete - } + err => InstructionError::ProgramFailedToComplete, }; Err(error) } diff --git a/programs/compute-budget/src/lib.rs b/programs/compute-budget/src/lib.rs index 6c866682c361bc..a12a41c0f6b696 100644 --- a/programs/compute-budget/src/lib.rs +++ b/programs/compute-budget/src/lib.rs @@ -1,9 +1,8 @@ -use { - solana_program_runtime::invoke_context::InvokeContext, - solana_sdk::{feature_set, instruction::InstructionError}, -}; +use {solana_program_runtime::invoke_context::InvokeContext, solana_sdk::feature_set}; -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +pub fn process_instruction( + invoke_context: &mut InvokeContext, +) -> Result<(), Box> { // Consume compute units if feature `native_programs_consume_cu` is activated, if invoke_context .feature_set diff --git a/programs/config/src/config_processor.rs b/programs/config/src/config_processor.rs index 3550d03f645f57..5de1d79a26cb7b 100644 --- a/programs/config/src/config_processor.rs +++ b/programs/config/src/config_processor.rs @@ -3,7 +3,7 @@ use { crate::ConfigKeys, bincode::deserialize, - solana_program_runtime::{ic_msg, invoke_context::InvokeContext}, + solana_program_runtime::{declare_process_instruction, ic_msg, invoke_context::InvokeContext}, solana_sdk::{ feature_set, instruction::InstructionError, program_utils::limited_deserialize, pubkey::Pubkey, transaction_context::IndexOfAccount, @@ -11,19 +11,14 @@ use { std::collections::BTreeSet, }; -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +declare_process_instruction!(450); +pub fn process_instruction_inner( + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let data = instruction_context.get_instruction_data(); - // Consume compute units if feature `native_programs_consume_cu` is activated, - if invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()) - { - invoke_context.consume_checked(450)?; - } - let key_list: ConfigKeys = limited_deserialize(data)?; let config_account_key = transaction_context.get_key_of_account_at_index( instruction_context.get_index_of_instruction_account_in_transaction(0)?, diff --git a/programs/loader-v3/src/lib.rs b/programs/loader-v3/src/lib.rs index 474454c1af4b0a..8e93621ec6a798 100644 --- a/programs/loader-v3/src/lib.rs +++ b/programs/loader-v3/src/lib.rs @@ -141,7 +141,7 @@ fn calculate_heap_cost(heap_size: u64, heap_cost: u64) -> u64 { pub fn create_vm<'a, 'b>( invoke_context: &'a mut InvokeContext<'b>, program: &'a VerifiedExecutable>, -) -> Result>, InstructionError> { +) -> Result>, Box> { let config = program.get_executable().get_config(); let compute_budget = invoke_context.get_compute_budget(); let heap_size = compute_budget.heap_size.unwrap_or(HEAP_LENGTH); @@ -164,14 +164,14 @@ pub fn create_vm<'a, 'b>( .and_then(|memory_mapping| EbpfVm::new(program, invoke_context, memory_mapping, stack_len)) .map_err(|err| { ic_logger_msg!(log_collector, "Failed to create SBF VM: {}", err); - InstructionError::ProgramEnvironmentSetupFailure + Box::new(InstructionError::ProgramEnvironmentSetupFailure) }) } fn execute( invoke_context: &mut InvokeContext, program: &VerifiedExecutable>, -) -> Result<(), InstructionError> { +) -> Result<(), Box> { let log_collector = invoke_context.get_log_collector(); let stack_height = invoke_context.get_stack_height(); let transaction_context = &invoke_context.transaction_context; @@ -214,7 +214,7 @@ fn execute( let error = status.into(); Err(error) } - ProgramResult::Err(_) => Err(InstructionError::ProgramFailedToComplete), + ProgramResult::Err(_) => Err(Box::new(InstructionError::ProgramFailedToComplete)), _ => Ok(()), } } @@ -531,7 +531,9 @@ pub fn process_instruction_transfer_authority( Ok(()) } -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +pub fn process_instruction( + invoke_context: &mut InvokeContext, +) -> Result<(), Box> { let use_jit = true; let log_collector = invoke_context.get_log_collector(); let transaction_context = &invoke_context.transaction_context; @@ -558,20 +560,21 @@ pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), Ins process_instruction_transfer_authority(invoke_context) } } + .map_err(|err| Box::new(err) as Box) } else { let program = instruction_context.try_borrow_last_program_account(transaction_context)?; if !loader_v3::check_id(program.get_owner()) { ic_logger_msg!(log_collector, "Program not owned by loader"); - return Err(InstructionError::InvalidAccountOwner); + return Err(Box::new(InstructionError::InvalidAccountOwner)); } if program.get_data().is_empty() { ic_logger_msg!(log_collector, "Program is uninitialized"); - return Err(InstructionError::InvalidAccountData); + return Err(Box::new(InstructionError::InvalidAccountData)); } let state = get_state(program.get_data())?; if !state.is_deployed { ic_logger_msg!(log_collector, "Program is not deployed"); - return Err(InstructionError::InvalidArgument); + return Err(Box::new(InstructionError::InvalidArgument)); } let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); let (loaded_program, load_program_metrics) = load_program_from_account( @@ -593,9 +596,11 @@ pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), Ins match &loaded_program.program { LoadedProgramType::FailedVerification | LoadedProgramType::Closed - | LoadedProgramType::DelayVisibility => Err(InstructionError::InvalidAccountData), + | LoadedProgramType::DelayVisibility => { + Err(Box::new(InstructionError::InvalidAccountData)) + } LoadedProgramType::Typed(executable) => execute(invoke_context, executable), - _ => Err(InstructionError::IncorrectProgramId), + _ => Err(Box::new(InstructionError::IncorrectProgramId)), } } } diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index c24bb6bd125c78..602ca21fb27d51 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -8,7 +8,8 @@ use { }, log::*, solana_program_runtime::{ - invoke_context::InvokeContext, sysvar_cache::get_sysvar_with_account_check, + declare_process_instruction, invoke_context::InvokeContext, + sysvar_cache::get_sysvar_with_account_check, }, solana_sdk::{ clock::Clock, @@ -51,21 +52,16 @@ fn get_optional_pubkey<'a>( ) } -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +declare_process_instruction!(750); +pub fn process_instruction_inner( + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let data = instruction_context.get_instruction_data(); trace!("process_instruction: {:?}", data); - // Consume compute units if feature `native_programs_consume_cu` is activated, - if invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()) - { - invoke_context.consume_checked(750)?; - } - let get_stake_account = || { let me = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; if *me.get_owner() != id() { diff --git a/programs/vote/src/vote_processor.rs b/programs/vote/src/vote_processor.rs index bcc3fb879d0947..c13a800db55b2f 100644 --- a/programs/vote/src/vote_processor.rs +++ b/programs/vote/src/vote_processor.rs @@ -5,7 +5,8 @@ use { log::*, solana_program::vote::{instruction::VoteInstruction, program::id, state::VoteAuthorize}, solana_program_runtime::{ - invoke_context::InvokeContext, sysvar_cache::get_sysvar_with_account_check, + declare_process_instruction, invoke_context::InvokeContext, + sysvar_cache::get_sysvar_with_account_check, }, solana_sdk::{ feature_set, @@ -55,23 +56,18 @@ fn process_authorize_with_seed_instruction( ) } -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +// Citing `runtime/src/block_cost_limit.rs`, vote has statically defined 2_100 +// units; can consume based on instructions in the future like `bpf_loader` does. +declare_process_instruction!(2_100); +pub fn process_instruction_inner( + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let data = instruction_context.get_instruction_data(); trace!("process_instruction: {:?}", data); - // Consume compute units if feature `native_programs_consume_cu` is activated, - // Citing `runtime/src/block_cost_limit.rs`, vote has statically defined 2_100 - // units; can consume based on instructions in the future like `bpf_loader` does. - if invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()) - { - invoke_context.consume_checked(2_100)?; - } - let mut me = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; if *me.get_owner() != id() { return Err(InstructionError::InvalidAccountOwner); diff --git a/programs/zk-token-proof/src/lib.rs b/programs/zk-token-proof/src/lib.rs index a9f064d5772e11..e6e5a916131217 100644 --- a/programs/zk-token-proof/src/lib.rs +++ b/programs/zk-token-proof/src/lib.rs @@ -2,7 +2,7 @@ use { bytemuck::Pod, - solana_program_runtime::{ic_msg, invoke_context::InvokeContext}, + solana_program_runtime::{declare_process_instruction, ic_msg, invoke_context::InvokeContext}, solana_sdk::{ feature_set, instruction::{InstructionError, TRANSACTION_LEVEL_STACK_HEIGHT}, @@ -114,7 +114,10 @@ fn process_close_proof_context(invoke_context: &mut InvokeContext) -> Result<(), Ok(()) } -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +declare_process_instruction!(0); +pub fn process_instruction_inner( + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { if invoke_context.get_stack_height() != TRANSACTION_LEVEL_STACK_HEIGHT { // Not supported as an inner instruction return Err(InstructionError::UnsupportedProgramId); @@ -137,21 +140,27 @@ pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), Ins } ProofInstruction::VerifyCloseAccount => { if native_programs_consume_cu { - invoke_context.consume_checked(6_012)?; + invoke_context + .consume_checked(6_012) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; } ic_msg!(invoke_context, "VerifyCloseAccount"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyWithdraw => { if native_programs_consume_cu { - invoke_context.consume_checked(112_454)?; + invoke_context + .consume_checked(112_454) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; } ic_msg!(invoke_context, "VerifyWithdraw"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyWithdrawWithheldTokens => { if native_programs_consume_cu { - invoke_context.consume_checked(7_943)?; + invoke_context + .consume_checked(7_943) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; } ic_msg!(invoke_context, "VerifyWithdrawWithheldTokens"); process_verify_proof::( @@ -160,21 +169,27 @@ pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), Ins } ProofInstruction::VerifyTransfer => { if native_programs_consume_cu { - invoke_context.consume_checked(219_290)?; + invoke_context + .consume_checked(219_290) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; } ic_msg!(invoke_context, "VerifyTransfer"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyTransferWithFee => { if native_programs_consume_cu { - invoke_context.consume_checked(407_121)?; + invoke_context + .consume_checked(407_121) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; } ic_msg!(invoke_context, "VerifyTransferWithFee"); process_verify_proof::(invoke_context) } ProofInstruction::VerifyPubkeyValidity => { if native_programs_consume_cu { - invoke_context.consume_checked(2_619)?; + invoke_context + .consume_checked(2_619) + .map_err(|_| InstructionError::ComputationalBudgetExceeded)?; } ic_msg!(invoke_context, "VerifyPubkeyValidity"); process_verify_proof::(invoke_context) diff --git a/runtime/benches/bank.rs b/runtime/benches/bank.rs index 91fd305f06ed00..dcb7a0d9148c80 100644 --- a/runtime/benches/bank.rs +++ b/runtime/benches/bank.rs @@ -15,7 +15,6 @@ use { client::{AsyncClient, SyncClient}, clock::MAX_RECENT_BLOCKHASHES, genesis_config::create_genesis_config, - instruction::InstructionError, message::Message, pubkey::Pubkey, signature::{Keypair, Signer}, @@ -35,8 +34,9 @@ const NOOP_PROGRAM_ID: [u8; 32] = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, ]; -#[allow(clippy::unnecessary_wraps)] -fn process_instruction(_invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +fn process_instruction( + _invoke_context: &mut InvokeContext, +) -> Result<(), Box> { Ok(()) } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index f48638ea754ea0..03dc0594a04425 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -1342,7 +1342,7 @@ fn test_rent_complex() { fn mock_process_instruction( invoke_context: &mut InvokeContext, - ) -> result::Result<(), InstructionError> { + ) -> result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -1359,7 +1359,7 @@ fn test_rent_complex() { } } } else { - Err(InstructionError::InvalidInstructionData) + Err(Box::new(InstructionError::InvalidInstructionData)) } } @@ -5072,14 +5072,14 @@ fn test_add_builtin() { } fn mock_vote_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let program_id = instruction_context.get_last_program_key(transaction_context)?; if mock_vote_program_id() != *program_id { - return Err(InstructionError::IncorrectProgramId); + return Err(Box::new(InstructionError::IncorrectProgramId)); } - Err(InstructionError::Custom(42)) + Err(Box::new(InstructionError::Custom(42))) } assert!(bank.get_account(&mock_vote_program_id()).is_none()); @@ -5130,10 +5130,10 @@ fn test_add_duplicate_static_program() { fn mock_vote_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { // mock builtin must consume units invoke_context.consume_checked(1)?; - Err(InstructionError::Custom(42)) + Err(Box::new(InstructionError::Custom(42))) } let mock_account = Keypair::new(); @@ -5180,10 +5180,10 @@ fn test_add_instruction_processor_for_existing_unrelated_accounts() { fn mock_ix_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { // mock builtin must consume units invoke_context.consume_checked(1)?; - Err(InstructionError::Custom(42)) + Err(Box::new(InstructionError::Custom(42))) } // Non-builtin loader accounts can not be used for instruction processing @@ -6472,7 +6472,7 @@ fn test_transaction_with_duplicate_accounts_in_instruction() { fn mock_process_instruction( invoke_context: &mut InvokeContext, - ) -> result::Result<(), InstructionError> { + ) -> result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -6533,7 +6533,7 @@ fn test_transaction_with_program_ids_passed_to_programs() { #[allow(clippy::unnecessary_wraps)] fn mock_process_instruction( invoke_context: &mut InvokeContext, - ) -> result::Result<(), InstructionError> { + ) -> result::Result<(), Box> { // mock builtin must consume units invoke_context.consume_checked(1)?; Ok(()) @@ -6753,7 +6753,7 @@ fn test_program_id_as_payer() { #[allow(clippy::unnecessary_wraps)] fn mock_ok_vote_processor( invoke_context: &mut InvokeContext, -) -> std::result::Result<(), InstructionError> { +) -> std::result::Result<(), Box> { // mock builtin must consume units invoke_context.consume_checked(1)?; Ok(()) @@ -7001,7 +7001,7 @@ fn test_bank_hash_consistency() { fn test_same_program_id_uses_unqiue_executable_accounts() { fn nested_processor( invoke_context: &mut InvokeContext, - ) -> result::Result<(), InstructionError> { + ) -> result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let _ = instruction_context @@ -7223,7 +7223,7 @@ fn test_add_builtin_no_overwrite() { #[allow(clippy::unnecessary_wraps)] fn mock_ix_processor( _invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { Ok(()) } @@ -7254,7 +7254,7 @@ fn test_add_builtin_loader_no_overwrite() { #[allow(clippy::unnecessary_wraps)] fn mock_ix_processor( _context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { Ok(()) } @@ -10002,7 +10002,7 @@ fn test_tx_return_data() { let mock_program_id = Pubkey::from([2u8; 32]); fn mock_process_instruction( invoke_context: &mut InvokeContext, - ) -> result::Result<(), InstructionError> { + ) -> result::Result<(), Box> { let mock_program_id = Pubkey::from([2u8; 32]); let transaction_context = &mut invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; @@ -10199,7 +10199,7 @@ fn test_transfer_sysvar() { fn mock_ix_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; // mock builtin should consume units @@ -10412,7 +10412,7 @@ fn test_compute_budget_program_noop() { fn mock_ix_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { let compute_budget = invoke_context.get_compute_budget(); assert_eq!( *compute_budget, @@ -10459,7 +10459,7 @@ fn test_compute_request_instruction() { fn mock_ix_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { let compute_budget = invoke_context.get_compute_budget(); assert_eq!( *compute_budget, @@ -10513,7 +10513,7 @@ fn test_failed_compute_request_instruction() { fn mock_ix_processor( invoke_context: &mut InvokeContext, - ) -> std::result::Result<(), InstructionError> { + ) -> std::result::Result<(), Box> { let compute_budget = invoke_context.get_compute_budget(); assert_eq!( *compute_budget, @@ -11078,7 +11078,7 @@ enum MockTransferInstruction { fn mock_transfer_process_instruction( invoke_context: &mut InvokeContext, -) -> result::Result<(), InstructionError> { +) -> result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -11097,7 +11097,7 @@ fn mock_transfer_process_instruction( } } } else { - Err(InstructionError::InvalidInstructionData) + Err(Box::new(InstructionError::InvalidInstructionData)) } } @@ -11890,7 +11890,7 @@ enum MockReallocInstruction { fn mock_realloc_process_instruction( invoke_context: &mut InvokeContext, -) -> result::Result<(), InstructionError> { +) -> result::Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -11929,7 +11929,7 @@ fn mock_realloc_process_instruction( } } } else { - Err(InstructionError::InvalidInstructionData) + Err(Box::new(InstructionError::InvalidInstructionData)) } } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index bc859970019adc..b1a6066e6ab8d7 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -219,7 +219,7 @@ mod tests { fn mock_system_process_instruction( invoke_context: &mut InvokeContext, - ) -> Result<(), InstructionError> { + ) -> Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -245,7 +245,7 @@ mod tests { } } } else { - Err(InstructionError::InvalidInstructionData) + Err(Box::new(InstructionError::InvalidInstructionData)) } } @@ -432,7 +432,7 @@ mod tests { fn mock_system_process_instruction( invoke_context: &mut InvokeContext, - ) -> Result<(), InstructionError> { + ) -> Result<(), Box> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -448,7 +448,7 @@ mod tests { let dup_account = instruction_context .try_borrow_instruction_account(transaction_context, 2)?; if from_account.get_lamports() != dup_account.get_lamports() { - return Err(InstructionError::InvalidArgument); + return Err(Box::new(InstructionError::InvalidArgument)); } Ok(()) } @@ -460,7 +460,7 @@ mod tests { .try_borrow_instruction_account(transaction_context, 2)? .get_lamports(); if lamports_a != lamports_b { - return Err(InstructionError::InvalidArgument); + return Err(Box::new(InstructionError::InvalidArgument)); } Ok(()) } @@ -479,7 +479,7 @@ mod tests { } } } else { - Err(InstructionError::InvalidInstructionData) + Err(Box::new(InstructionError::InvalidInstructionData)) } } @@ -647,10 +647,10 @@ mod tests { let mock_program_id = Pubkey::new_unique(); fn mock_process_instruction( invoke_context: &mut InvokeContext, - ) -> Result<(), InstructionError> { + ) -> Result<(), Box> { // mock builtin should consume units let _ = invoke_context.consume_checked(1); - Err(InstructionError::Custom(0xbabb1e)) + Err(Box::new(InstructionError::Custom(0xbabb1e))) } let builtin_programs = &[BuiltinProgram { program_id: mock_program_id, diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index 058e91524066ec..3ca5c1ded1a1b1 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -5,7 +5,8 @@ use { }, log::*, solana_program_runtime::{ - ic_msg, invoke_context::InvokeContext, sysvar_cache::get_sysvar_with_account_check, + declare_process_instruction, ic_msg, invoke_context::InvokeContext, + sysvar_cache::get_sysvar_with_account_check, }, solana_sdk::{ account::AccountSharedData, @@ -315,7 +316,10 @@ fn transfer_with_seed( ) } -pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), InstructionError> { +declare_process_instruction!(150); +pub fn process_instruction_inner( + invoke_context: &mut InvokeContext, +) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; let instruction_data = instruction_context.get_instruction_data(); @@ -323,14 +327,6 @@ pub fn process_instruction(invoke_context: &mut InvokeContext) -> Result<(), Ins trace!("process_instruction: {:?}", instruction); - // Consume compute units if feature `native_programs_consume_cu` is activated, - if invoke_context - .feature_set - .is_active(&feature_set::native_programs_consume_cu::id()) - { - invoke_context.consume_checked(150)?; - } - let signers = instruction_context.get_signers(transaction_context)?; match instruction { SystemInstruction::CreateAccount { From 6d22d4a559a63bc07b15eb348a9a12343304ce75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 13 Mar 2023 16:38:50 +0100 Subject: [PATCH 3/8] Bump to solana_rbpf v0.3.0 --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- programs/bpf_loader/src/lib.rs | 2 +- programs/sbf/Cargo.lock | 4 ++-- programs/sbf/Cargo.toml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 14e7d8b6e32e5b..b769c21b687f10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7343,9 +7343,9 @@ dependencies = [ [[package]] name = "solana_rbpf" -version = "0.2.40" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a5735b8c9defc3723162321a61ef738d34168401eeef213f62a32809739b0f5" +checksum = "edb31627f86190e2d97f86988f1de1a757b530caa50694244d9d28812611b063" dependencies = [ "byteorder", "combine", diff --git a/Cargo.toml b/Cargo.toml index 9e297d2ebb0ca1..714fe2862f193c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -289,7 +289,7 @@ signal-hook = "0.3.14" smpl_jwt = "0.7.1" socket2 = "0.4.7" soketto = "0.7" -solana_rbpf = "=0.2.40" +solana_rbpf = "=0.3.0" solana-account-decoder = { path = "account-decoder", version = "=1.16.0" } solana-address-lookup-table-program = { path = "programs/address-lookup-table", version = "=1.16.0" } solana-banks-client = { path = "banks-client", version = "=1.16.0" } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index f0624f3eb9645b..dad968de03efa0 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1751,7 +1751,7 @@ mod tests { } #[test] - #[should_panic(expected = "ExceededMaxInstructions(31, 10)")] + #[should_panic(expected = "ExceededMaxInstructions(31)")] fn test_bpf_loader_non_terminating_program() { #[rustfmt::skip] let program = &[ diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 96b7fcb902ab69..b113edf93d97e4 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -6497,9 +6497,9 @@ dependencies = [ [[package]] name = "solana_rbpf" -version = "0.2.40" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a5735b8c9defc3723162321a61ef738d34168401eeef213f62a32809739b0f5" +checksum = "edb31627f86190e2d97f86988f1de1a757b530caa50694244d9d28812611b063" dependencies = [ "byteorder 1.4.3", "combine", diff --git a/programs/sbf/Cargo.toml b/programs/sbf/Cargo.toml index a413ef7063da43..99c2bae1e54f99 100644 --- a/programs/sbf/Cargo.toml +++ b/programs/sbf/Cargo.toml @@ -24,7 +24,7 @@ num-traits = "0.2" rand = "0.7" serde = "1.0.112" serde_json = "1.0.56" -solana_rbpf = "=0.2.40" +solana_rbpf = "=0.3.0" solana-account-decoder = { path = "../../account-decoder", version = "=1.16.0" } solana-address-lookup-table-program = { path = "../../programs/address-lookup-table", version = "=1.16.0" } solana-bpf-loader-program = { path = "../bpf_loader", version = "=1.16.0" } From 167ff2d35517944a69d6d2f2cc6e2d289c6c3e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 13 Mar 2023 16:38:50 +0100 Subject: [PATCH 4/8] Removes Result from return type of EbpfVm::new(). --- programs/bpf_loader/src/lib.rs | 7 ++++++- programs/loader-v3/src/lib.rs | 16 ++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index dad968de03efa0..82046c751a5757 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -369,7 +369,12 @@ pub fn create_ebpf_vm<'a, 'b>( None, )?; - EbpfVm::new(program, invoke_context, memory_mapping, stack_len) + Ok(EbpfVm::new( + program, + invoke_context, + memory_mapping, + stack_len, + )) } #[macro_export] diff --git a/programs/loader-v3/src/lib.rs b/programs/loader-v3/src/lib.rs index 8e93621ec6a798..0cbdeee3f349d9 100644 --- a/programs/loader-v3/src/lib.rs +++ b/programs/loader-v3/src/lib.rs @@ -160,12 +160,16 @@ pub fn create_vm<'a, 'b>( MemoryRegion::new_writable(heap.as_slice_mut(), ebpf::MM_HEAP_START), ]; let log_collector = invoke_context.get_log_collector(); - MemoryMapping::new(regions, config) - .and_then(|memory_mapping| EbpfVm::new(program, invoke_context, memory_mapping, stack_len)) - .map_err(|err| { - ic_logger_msg!(log_collector, "Failed to create SBF VM: {}", err); - Box::new(InstructionError::ProgramEnvironmentSetupFailure) - }) + let memory_mapping = MemoryMapping::new(regions, config).map_err(|err| { + ic_logger_msg!(log_collector, "Failed to create SBF VM: {}", err); + Box::new(InstructionError::ProgramEnvironmentSetupFailure) + })?; + Ok(EbpfVm::new( + program, + invoke_context, + memory_mapping, + stack_len, + )) } fn execute( From b3f7ee50807e54c672f8d1f1ca2db2ddc7ae5139 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 5 Apr 2023 08:40:30 +0200 Subject: [PATCH 5/8] Turns EbpfError into Box. --- program-runtime/src/loaded_programs.rs | 2 +- programs/bpf_loader/src/lib.rs | 79 ++++------ programs/bpf_loader/src/syscalls/cpi.rs | 68 +++++---- programs/bpf_loader/src/syscalls/logging.rs | 10 +- programs/bpf_loader/src/syscalls/mem_ops.rs | 13 +- programs/bpf_loader/src/syscalls/mod.rs | 161 ++++++++------------ programs/bpf_loader/src/syscalls/sysvar.rs | 10 +- programs/loader-v3/src/lib.rs | 6 +- 8 files changed, 151 insertions(+), 198 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index c9fad65c705344..9a5c795110cc9b 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -137,7 +137,7 @@ impl LoadedProgram { account_size: usize, use_jit: bool, metrics: &mut LoadProgramMetrics, - ) -> Result { + ) -> Result> { let mut load_elf_time = Measure::start("load_elf_time"); let executable = Executable::load(elf_bytes, loader.clone())?; load_elf_time.stop(); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 82046c751a5757..cf3455fd50ba9d 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -10,11 +10,7 @@ pub mod upgradeable_with_jit; pub mod with_jit; use { - crate::{ - allocator_bump::BpfAllocator, - serialization::{deserialize_parameters, serialize_parameters}, - syscalls::SyscallError, - }, + crate::{allocator_bump::BpfAllocator, syscalls::SyscallError}, solana_measure::measure::Measure, solana_program_runtime::{ compute_budget::ComputeBudget, @@ -30,7 +26,7 @@ use { aligned_memory::AlignedMemory, ebpf::{self, HOST_ALIGN, MM_HEAP_START}, elf::Executable, - error::{EbpfError, UserDefinedError}, + error::UserDefinedError, memory_region::{MemoryCowCallback, MemoryMapping, MemoryRegion}, verifier::{RequisiteVerifier, VerifierError}, vm::{ContextObject, EbpfVm, ProgramResult, VerifiedExecutable}, @@ -326,7 +322,7 @@ pub fn create_ebpf_vm<'a, 'b>( regions: Vec, orig_account_lengths: Vec, invoke_context: &'a mut InvokeContext<'b>, -) -> Result>, EbpfError> { +) -> Result>, Box> { let round_up_heap_size = invoke_context .feature_set .is_active(&round_up_heap_size::id()); @@ -411,7 +407,7 @@ fn create_memory_mapping<'a, 'b, C: ContextObject>( heap: &'b mut AlignedMemory<{ HOST_ALIGN }>, additional_regions: Vec, cow_cb: Option, -) -> Result, EbpfError> { +) -> Result, Box> { let config = executable.get_config(); let regions: Vec = vec![ executable.get_ro_region(), @@ -441,20 +437,18 @@ pub fn process_instruction( invoke_context: &mut InvokeContext, ) -> Result<(), Box> { process_instruction_inner(invoke_context, false) - .map_err(|err| Box::new(err) as Box) } pub fn process_instruction_jit( invoke_context: &mut InvokeContext, ) -> Result<(), Box> { process_instruction_inner(invoke_context, true) - .map_err(|err| Box::new(err) as Box) } fn process_instruction_inner( invoke_context: &mut InvokeContext, use_jit: bool, -) -> Result<(), InstructionError> { +) -> Result<(), Box> { let log_collector = invoke_context.get_log_collector(); let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; @@ -529,7 +523,7 @@ fn process_instruction_inner( )?; if first_account.is_executable() { ic_logger_msg!(log_collector, "BPF loader is executable"); - return Err(InstructionError::IncorrectProgramId); + return Err(Box::new(InstructionError::IncorrectProgramId)); } } } @@ -565,13 +559,14 @@ fn process_instruction_inner( } else { ic_logger_msg!(log_collector, "Invalid BPF loader id"); Err(InstructionError::IncorrectProgramId) - }; + } + .map_err(|error| Box::new(error) as Box); } // Program Invocation if !program_account.is_executable() { ic_logger_msg!(log_collector, "Program is not executable"); - return Err(InstructionError::IncorrectProgramId); + return Err(Box::new(InstructionError::IncorrectProgramId)); } let programdata_account = if bpf_loader_upgradeable::check_id(program_account.get_owner()) { @@ -612,10 +607,10 @@ fn process_instruction_inner( match &executor.program { LoadedProgramType::FailedVerification | LoadedProgramType::Closed - | LoadedProgramType::DelayVisibility => Err(InstructionError::InvalidAccountData), + | LoadedProgramType::DelayVisibility => Err(Box::new(InstructionError::InvalidAccountData)), LoadedProgramType::LegacyV0(executable) => execute(executable, invoke_context), LoadedProgramType::LegacyV1(executable) => execute(executable, invoke_context), - _ => Err(InstructionError::IncorrectProgramId), + _ => Err(Box::new(InstructionError::IncorrectProgramId)), } } @@ -1536,7 +1531,7 @@ fn process_loader_instruction( fn execute<'a, 'b: 'a>( executable: &'a VerifiedExecutable>, invoke_context: &'a mut InvokeContext<'b>, -) -> Result<(), InstructionError> { +) -> Result<(), Box> { let log_collector = invoke_context.get_log_collector(); let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; @@ -1547,7 +1542,7 @@ fn execute<'a, 'b: 'a>( let use_jit = executable.get_executable().get_compiled_program().is_some(); let mut serialize_time = Measure::start("serialize"); - let (parameter_bytes, regions, account_lengths) = serialize_parameters( + let (parameter_bytes, regions, account_lengths) = serialization::serialize_parameters( invoke_context.transaction_context, instruction_context, invoke_context @@ -1579,7 +1574,7 @@ fn execute<'a, 'b: 'a>( Ok(info) => info, Err(e) => { ic_logger_msg!(log_collector, "Failed to create SBF VM: {}", e); - return Err(InstructionError::ProgramEnvironmentSetupFailure); + return Err(Box::new(InstructionError::ProgramEnvironmentSetupFailure)); } }; create_vm_time.stop(); @@ -1618,45 +1613,32 @@ fn execute<'a, 'b: 'a>( } else { status.into() }; - Err(error) - } - ProgramResult::Err(error) => { - let error = match error { - /*EbpfError::UserError(user_error) if let BpfError::SyscallError( - SyscallError::InstructionError(instruction_error), - ) = user_error.downcast_ref::().unwrap() => instruction_error.clone(),*/ - EbpfError::UserError(user_error) - if matches!( - user_error.downcast_ref::().unwrap(), - BpfError::SyscallError(SyscallError::InstructionError(_)), - ) => - { - match user_error.downcast_ref::().unwrap() { - BpfError::SyscallError(SyscallError::InstructionError( - instruction_error, - )) => instruction_error.clone(), - _ => unreachable!(), - } - } - err => InstructionError::ProgramFailedToComplete, - }; - Err(error) + Err(Box::new(error) as Box) } + ProgramResult::Err(error) => Err(error), _ => Ok(()), } }; execute_time.stop(); - let mut deserialize_time = Measure::start("deserialize"); - let execute_or_deserialize_result = execution_result.and_then(|_| { - deserialize_parameters( + fn deserialize_parameters( + invoke_context: &mut InvokeContext, + parameter_bytes: &[u8], + ) -> Result<(), InstructionError> { + serialization::deserialize_parameters( invoke_context.transaction_context, invoke_context .transaction_context .get_current_instruction_context()?, - parameter_bytes.as_slice(), + parameter_bytes, invoke_context.get_orig_account_lengths()?, ) + } + + let mut deserialize_time = Measure::start("deserialize"); + let execute_or_deserialize_result = execution_result.and_then(|_| { + deserialize_parameters(invoke_context, parameter_bytes.as_slice()) + .map_err(|error| Box::new(error) as Box) }); deserialize_time.stop(); @@ -1681,7 +1663,7 @@ mod tests { solana_rbpf::{ ebpf::MM_INPUT_START, elf::Executable, - verifier::Verifier, + verifier::{Verifier, VerifierError}, vm::{BuiltInProgram, Config, ContextObject, FunctionRegistry}, }, solana_sdk::{ @@ -1799,8 +1781,7 @@ mod tests { &mut context_object, memory_mapping, stack_len, - ) - .unwrap(); + ); vm.execute_program(true).1.unwrap(); } diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 41315377c52c6c..5c295f75a79cd1 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -39,7 +39,7 @@ impl<'a> CallerAccount<'a> { _vm_addr: u64, account_info: &AccountInfo, original_data_len: usize, - ) -> Result, EbpfError> { + ) -> Result, Box> { // account_info points to host memory. The addresses used internally are // in vm space so they need to be translated. @@ -129,7 +129,7 @@ impl<'a> CallerAccount<'a> { vm_addr: u64, account_info: &SolAccountInfo, original_data_len: usize, - ) -> Result, EbpfError> { + ) -> Result, Box> { // account_info points to host memory. The addresses used internally are // in vm space so they need to be translated. @@ -212,7 +212,7 @@ trait SyscallInvokeSigned { addr: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result; + ) -> Result>; fn translate_accounts<'a>( instruction_accounts: &[InstructionAccount], program_indices: &[IndexOfAccount], @@ -220,14 +220,14 @@ trait SyscallInvokeSigned { account_infos_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result, EbpfError>; + ) -> Result, Box>; fn translate_signers( program_id: &Pubkey, signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &InvokeContext, - ) -> Result, EbpfError>; + ) -> Result, Box>; } declare_syscall!( @@ -241,7 +241,7 @@ declare_syscall!( signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { cpi_common::( invoke_context, instruction_addr, @@ -259,7 +259,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { addr: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result { + ) -> Result> { let ix = translate_type::( memory_mapping, addr, @@ -312,7 +312,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { account_infos_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result, EbpfError> { + ) -> Result, Box> { let (account_infos, account_info_keys) = translate_account_infos( account_infos_addr, account_infos_len, @@ -339,7 +339,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &InvokeContext, - ) -> Result, EbpfError> { + ) -> Result, Box> { let mut signers = Vec::new(); if signers_seeds_len > 0 { let signers_seeds = translate_slice::<&[&[u8]]>( @@ -377,7 +377,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { invoke_context.get_check_size(), ) }) - .collect::, EbpfError>>()?; + .collect::, Box>>()?; let signer = Pubkey::create_program_address(&seeds, program_id) .map_err(SyscallError::BadSeeds)?; signers.push(signer); @@ -453,7 +453,7 @@ declare_syscall!( signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { cpi_common::( invoke_context, instruction_addr, @@ -471,7 +471,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { addr: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result { + ) -> Result> { let ix_c = translate_type::( memory_mapping, addr, @@ -530,7 +530,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { is_writable: meta_c.is_writable, }) }) - .collect::, EbpfError>>()?; + .collect::, Box>>()?; Ok(StableInstruction { accounts: accounts.into(), @@ -546,7 +546,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { account_infos_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result, EbpfError> { + ) -> Result, Box> { let (account_infos, account_info_keys) = translate_account_infos( account_infos_addr, account_infos_len, @@ -573,7 +573,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &InvokeContext, - ) -> Result, EbpfError> { + ) -> Result, Box> { if signers_seeds_len > 0 { let signers_seeds = translate_slice::( memory_mapping, @@ -612,11 +612,11 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { invoke_context.get_check_size(), ) }) - .collect::, EbpfError>>()?; + .collect::, Box>>()?; Pubkey::create_program_address(&seeds_bytes, program_id) .map_err(|err| SyscallError::BadSeeds(err).into()) }) - .collect::, EbpfError>>()?) + .collect::, Box>>()?) } else { Ok(vec![]) } @@ -629,7 +629,7 @@ fn translate_account_infos<'a, T, F>( key_addr: F, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, -) -> Result<(&'a [T], Vec<&'a Pubkey>), EbpfError> +) -> Result<(&'a [T], Vec<&'a Pubkey>), Box> where F: Fn(&T) -> u64, { @@ -650,7 +650,7 @@ where invoke_context.get_check_aligned(), ) }) - .collect::, EbpfError>>()?; + .collect::, Box>>()?; Ok((account_infos, account_info_keys)) } @@ -666,9 +666,15 @@ fn translate_and_update_accounts<'a, T, F>( invoke_context: &mut InvokeContext, memory_mapping: &MemoryMapping, do_translate: F, -) -> Result, EbpfError> +) -> Result, Box> where - F: Fn(&InvokeContext, &MemoryMapping, u64, &T, usize) -> Result, EbpfError>, + F: Fn( + &InvokeContext, + &MemoryMapping, + u64, + &T, + usize, + ) -> Result, Box>, { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context @@ -770,7 +776,7 @@ fn check_instruction_size( num_accounts: usize, data_len: usize, invoke_context: &mut InvokeContext, -) -> Result<(), EbpfError> { +) -> Result<(), Box> { if invoke_context .feature_set .is_active(&feature_set::loosen_cpi_size_restriction::id()) @@ -809,7 +815,7 @@ fn check_instruction_size( fn check_account_infos( num_account_infos: usize, invoke_context: &mut InvokeContext, -) -> Result<(), EbpfError> { +) -> Result<(), Box> { if invoke_context .feature_set .is_active(&feature_set::loosen_cpi_size_restriction::id()) @@ -847,7 +853,7 @@ fn check_authorized_program( program_id: &Pubkey, instruction_data: &[u8], invoke_context: &InvokeContext, -) -> Result<(), EbpfError> { +) -> Result<(), Box> { if native_loader::check_id(program_id) || bpf_loader::check_id(program_id) || bpf_loader_deprecated::check_id(program_id) @@ -879,7 +885,7 @@ fn cpi_common( signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, -) -> Result { +) -> Result> { // CPI entry. // // Translate the inputs to the syscall and synchronize the caller's account @@ -967,7 +973,7 @@ fn update_callee_account( invoke_context: &InvokeContext, caller_account: &CallerAccount, mut callee_account: BorrowedAccount<'_>, -) -> Result<(), EbpfError> { +) -> Result<(), Box> { let is_disable_cpi_setting_executable_and_rent_epoch_active = invoke_context .feature_set .is_active(&disable_cpi_setting_executable_and_rent_epoch::id()); @@ -987,9 +993,7 @@ fn update_callee_account( .set_data_from_slice(caller_account.data) .map_err(SyscallError::InstructionError)?, Err(err) if callee_account.get_data() != caller_account.data => { - return Err(EbpfError::UserError(Box::new(BpfError::SyscallError( - SyscallError::InstructionError(err), - )))); + return Err(Box::new(err)); } _ => {} } @@ -1048,7 +1052,7 @@ fn update_caller_account( memory_mapping: &MemoryMapping, caller_account: &mut CallerAccount, callee_account: &BorrowedAccount<'_>, -) -> Result<(), EbpfError> { +) -> Result<(), Box> { *caller_account.lamports = callee_account.get_lamports(); *caller_account.owner = *callee_account.get_owner(); let new_len = callee_account.get_data().len(); @@ -1449,9 +1453,7 @@ mod tests { &mut caller_account, &callee_account, ), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::InvalidRealloc) - ) + Err(error) if error.downcast_ref::().unwrap() == &InstructionError::InvalidRealloc, )); } diff --git a/programs/bpf_loader/src/syscalls/logging.rs b/programs/bpf_loader/src/syscalls/logging.rs index b741f49a943939..48dd7f670c21b7 100644 --- a/programs/bpf_loader/src/syscalls/logging.rs +++ b/programs/bpf_loader/src/syscalls/logging.rs @@ -11,7 +11,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context .get_compute_budget() .syscall_base_cost @@ -47,7 +47,7 @@ declare_syscall!( arg4: u64, arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context.get_compute_budget().log_64_units; consume_compute_meter(invoke_context, cost)?; @@ -70,7 +70,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context.get_compute_budget().syscall_base_cost; consume_compute_meter(invoke_context, cost)?; @@ -94,7 +94,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context.get_compute_budget().log_pubkey_units; consume_compute_meter(invoke_context, cost)?; @@ -119,7 +119,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; diff --git a/programs/bpf_loader/src/syscalls/mem_ops.rs b/programs/bpf_loader/src/syscalls/mem_ops.rs index a424bdb0e1f849..5460ccc158bfab 100644 --- a/programs/bpf_loader/src/syscalls/mem_ops.rs +++ b/programs/bpf_loader/src/syscalls/mem_ops.rs @@ -1,6 +1,9 @@ use {super::*, crate::declare_syscall}; -fn mem_op_consume(invoke_context: &mut InvokeContext, n: u64) -> Result<(), EbpfError> { +fn mem_op_consume( + invoke_context: &mut InvokeContext, + n: u64, +) -> Result<(), Box> { let compute_budget = invoke_context.get_compute_budget(); let cost = compute_budget .mem_op_base_cost @@ -19,7 +22,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { mem_op_consume(invoke_context, n)?; if !is_nonoverlapping(src_addr, n, dst_addr, n) { @@ -66,7 +69,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { mem_op_consume(invoke_context, n)?; let dst = translate_slice_mut::( @@ -101,7 +104,7 @@ declare_syscall!( cmp_result_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { mem_op_consume(invoke_context, n)?; let s1 = translate_slice::( @@ -149,7 +152,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { mem_op_consume(invoke_context, n)?; let s = translate_slice_mut::( diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 7cc6b162a368d8..66fe795c38c36f 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -133,7 +133,10 @@ impl From for EbpfError { } } -fn consume_compute_meter(invoke_context: &InvokeContext, amount: u64) -> Result<(), EbpfError> { +fn consume_compute_meter( + invoke_context: &InvokeContext, + amount: u64, +) -> Result<(), Box> { invoke_context .consume_checked(amount) .map_err(SyscallError::InstructionError)?; @@ -156,7 +159,7 @@ pub fn create_loader<'a>( reject_deployment_of_broken_elfs: bool, disable_deploy_of_alloc_free_syscall: bool, debugging_features: bool, -) -> Result>>, EbpfError> { +) -> Result>>, Box> { use rand::Rng; let config = Config { max_call_depth: compute_budget.max_call_depth, @@ -330,7 +333,7 @@ fn translate( access_type: AccessType, vm_addr: u64, len: u64, -) -> Result { +) -> Result> { memory_mapping.map(access_type, vm_addr, len, 0).into() } @@ -339,7 +342,7 @@ fn translate_type_inner<'a, T>( access_type: AccessType, vm_addr: u64, check_aligned: bool, -) -> Result<&'a mut T, EbpfError> { +) -> Result<&'a mut T, Box> { let host_addr = translate(memory_mapping, access_type, vm_addr, size_of::() as u64)?; if check_aligned && (host_addr as *mut T as usize).wrapping_rem(align_of::()) != 0 { @@ -351,14 +354,14 @@ fn translate_type_mut<'a, T>( memory_mapping: &MemoryMapping, vm_addr: u64, check_aligned: bool, -) -> Result<&'a mut T, EbpfError> { +) -> Result<&'a mut T, Box> { translate_type_inner::(memory_mapping, AccessType::Store, vm_addr, check_aligned) } fn translate_type<'a, T>( memory_mapping: &MemoryMapping, vm_addr: u64, check_aligned: bool, -) -> Result<&'a T, EbpfError> { +) -> Result<&'a T, Box> { translate_type_inner::(memory_mapping, AccessType::Load, vm_addr, check_aligned) .map(|value| &*value) } @@ -370,7 +373,7 @@ fn translate_slice_inner<'a, T>( len: u64, check_aligned: bool, check_size: bool, -) -> Result<&'a mut [T], EbpfError> { +) -> Result<&'a mut [T], Box> { if len == 0 { return Ok(&mut []); } @@ -393,7 +396,7 @@ fn translate_slice_mut<'a, T>( len: u64, check_aligned: bool, check_size: bool, -) -> Result<&'a mut [T], EbpfError> { +) -> Result<&'a mut [T], Box> { translate_slice_inner::( memory_mapping, AccessType::Store, @@ -409,7 +412,7 @@ fn translate_slice<'a, T>( len: u64, check_aligned: bool, check_size: bool, -) -> Result<&'a [T], EbpfError> { +) -> Result<&'a [T], Box> { translate_slice_inner::( memory_mapping, AccessType::Load, @@ -430,8 +433,8 @@ fn translate_string_and_do( check_aligned: bool, check_size: bool, stop_truncating_strings_in_syscalls: bool, - work: &mut dyn FnMut(&str) -> Result, -) -> Result { + work: &mut dyn FnMut(&str) -> Result>, +) -> Result> { let buf = translate_slice::(memory_mapping, addr, len, check_aligned, check_size)?; let msg = if stop_truncating_strings_in_syscalls { buf @@ -488,7 +491,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { Err(SyscallError::Abort.into()) } ); @@ -505,7 +508,7 @@ declare_syscall!( column: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { consume_compute_meter(invoke_context, len)?; translate_string_and_do( @@ -538,7 +541,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let allocator = invoke_context .get_allocator() .map_err(SyscallError::InstructionError)?; @@ -576,7 +579,7 @@ fn translate_and_check_program_address_inputs<'a>( memory_mapping: &mut MemoryMapping, check_aligned: bool, check_size: bool, -) -> Result<(Vec<&'a [u8]>, &'a Pubkey), EbpfError> { +) -> Result<(Vec<&'a [u8]>, &'a Pubkey), Box> { let untranslated_seeds = translate_slice::<&[&u8]>( memory_mapping, seeds_addr, @@ -601,7 +604,7 @@ fn translate_and_check_program_address_inputs<'a>( check_size, ) }) - .collect::, EbpfError>>()?; + .collect::, Box>>()?; let program_id = translate_type::(memory_mapping, program_id_addr, check_aligned)?; Ok((seeds, program_id)) } @@ -617,7 +620,7 @@ declare_syscall!( address_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context .get_compute_budget() .create_program_address_units; @@ -661,7 +664,7 @@ declare_syscall!( address_addr: u64, bump_seed_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context .get_compute_budget() .create_program_address_units; @@ -731,7 +734,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let compute_budget = invoke_context.get_compute_budget(); if compute_budget.sha256_max_slices < vals_len { ic_msg!( @@ -794,7 +797,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let compute_budget = invoke_context.get_compute_budget(); if compute_budget.sha256_max_slices < vals_len { ic_msg!( @@ -857,7 +860,7 @@ declare_syscall!( result_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let cost = invoke_context.get_compute_budget().secp256k1_recover_cost; consume_compute_meter(invoke_context, cost)?; @@ -949,7 +952,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { use solana_zk_token_sdk::curve25519::{curve_syscall_traits::*, edwards, ristretto}; match curve_id { CURVE25519_EDWARDS => { @@ -1006,7 +1009,7 @@ declare_syscall!( right_input_addr: u64, result_point_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { use solana_zk_token_sdk::curve25519::{ curve_syscall_traits::*, edwards, ristretto, scalar, }; @@ -1207,7 +1210,7 @@ declare_syscall!( points_len: u64, result_point_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { use solana_zk_token_sdk::curve25519::{ curve_syscall_traits::*, edwards, ristretto, scalar, }; @@ -1310,7 +1313,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let compute_budget = invoke_context.get_compute_budget(); if compute_budget.sha256_max_slices < vals_len { ic_msg!( @@ -1373,7 +1376,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let budget = invoke_context.get_compute_budget(); let cost = len @@ -1424,7 +1427,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; @@ -1491,7 +1494,7 @@ declare_syscall!( data_addr: u64, accounts_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; @@ -1643,7 +1646,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; @@ -1663,7 +1666,7 @@ declare_syscall!( result_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { use solana_sdk::alt_bn128::prelude::{ALT_BN128_ADD, ALT_BN128_MUL, ALT_BN128_PAIRING}; let budget = invoke_context.get_compute_budget(); let (cost, output): (u64, usize) = match group_op { @@ -1748,7 +1751,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { let params = &translate_slice::( memory_mapping, params, @@ -1825,6 +1828,7 @@ mod tests { solana_rbpf::{ aligned_memory::AlignedMemory, ebpf::{self, HOST_ALIGN}, + error::EbpfError, memory_region::MemoryRegion, vm::{BuiltInFunction, Config}, }, @@ -1843,11 +1847,10 @@ mod tests { macro_rules! assert_access_violation { ($result:expr, $va:expr, $len:expr) => { - match $result { - ProgramResult::Err(EbpfError::AccessViolation(_, _, va, len, _)) - if $va == va && $len == len => {} - ProgramResult::Err(EbpfError::StackAccessViolation(_, _, va, len, _)) - if $va == va && $len == len => {} + match $result.unwrap_err().downcast_ref::().unwrap() { + EbpfError::AccessViolation(_, _, va, len, _) if $va == *va && $len == *len => {} + EbpfError::StackAccessViolation(_, _, va, len, _) if $va == *va && $len == *len => { + } _ => panic!(), } }; @@ -2066,7 +2069,7 @@ mod tests { } #[test] - #[should_panic(expected = "UserError(SyscallError(Abort))")] + #[should_panic(expected = "Abort")] fn test_syscall_abort() { prepare_mockup!(invoke_context, program_id, bpf_loader::id()); let config = Config::default(); @@ -2086,7 +2089,7 @@ mod tests { } #[test] - #[should_panic(expected = "UserError(SyscallError(Panic(\"Gaggablaghblagh!\", 42, 84)))")] + #[should_panic(expected = "Panic(\"Gaggablaghblagh!\", 42, 84)")] fn test_syscall_sol_panic() { prepare_mockup!(invoke_context, program_id, bpf_loader::id()); @@ -2112,9 +2115,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); invoke_context.mock_set_remaining(string.len() as u64); @@ -2195,9 +2196,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); assert_eq!( @@ -2281,9 +2280,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); invoke_context.mock_set_remaining(cost); @@ -2617,7 +2614,7 @@ mod tests { &mut result, ); assert_access_violation!(result, rw_va - 1, HASH_BYTES as u64); - + let mut result = ProgramResult::Ok(0); SyscallSha256::call( &mut invoke_context, ro_va, @@ -2630,9 +2627,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); } @@ -2710,9 +2705,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); } @@ -2790,9 +2783,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); } @@ -2962,9 +2953,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); } @@ -3136,9 +3125,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); } @@ -3479,7 +3466,7 @@ mod tests { program_id: &Pubkey, overlap_outputs: bool, syscall: BuiltInFunction>, - ) -> Result<(Pubkey, u8), EbpfError> { + ) -> Result<(Pubkey, u8), Box> { const SEEDS_VA: u64 = 0x100000000; const PROGRAM_ID_VA: u64 = 0x200000000; const ADDRESS_VA: u64 = 0x300000000; @@ -3526,14 +3513,14 @@ mod tests { &mut memory_mapping, &mut result, ); - Result::::from(result).map(|_| (address, bump_seed)) + Result::>::from(result).map(|_| (address, bump_seed)) } fn create_program_address( invoke_context: &mut InvokeContext, seeds: &[&[u8]], address: &Pubkey, - ) -> Result { + ) -> Result> { let (address, _) = call_program_address_common( invoke_context, seeds, @@ -3548,7 +3535,7 @@ mod tests { invoke_context: &mut InvokeContext, seeds: &[&[u8]], address: &Pubkey, - ) -> Result<(Pubkey, u8), EbpfError> { + ) -> Result<(Pubkey, u8), Box> { call_program_address_common( invoke_context, seeds, @@ -3621,9 +3608,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::CopyOverlapping - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::CopyOverlapping, )); } @@ -3775,9 +3760,7 @@ mod tests { ); assert!(matches!( result, - ProgramResult::Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::CopyOverlapping - ), + ProgramResult::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::CopyOverlapping, )); } @@ -3791,9 +3774,7 @@ mod tests { let exceeded_seed = &[127; MAX_SEED_LEN + 1]; assert!(matches!( create_program_address(&mut invoke_context, &[exceeded_seed], &address), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded), )); assert!(matches!( create_program_address( @@ -3801,9 +3782,7 @@ mod tests { &[b"short_seed", exceeded_seed], &address, ), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded), )); let max_seed = &[0; MAX_SEED_LEN]; assert!(create_program_address(&mut invoke_context, &[max_seed], &address).is_ok()); @@ -3847,9 +3826,7 @@ mod tests { ]; assert!(matches!( create_program_address(&mut invoke_context, max_seeds, &address), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded), )); assert_eq!( create_program_address(&mut invoke_context, &[b"", &[1]], &address).unwrap(), @@ -3886,9 +3863,7 @@ mod tests { invoke_context.mock_set_remaining(0); assert!(matches!( create_program_address(&mut invoke_context, &[b"", &[1]], &address), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); } @@ -3927,18 +3902,14 @@ mod tests { invoke_context.mock_set_remaining(cost * (max_tries - bump_seed as u64 - 1)); assert!(matches!( try_find_program_address(&mut invoke_context, seeds, &address), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &InstructionError::ComputationalBudgetExceeded, )); let exceeded_seed = &[127; MAX_SEED_LEN + 1]; invoke_context.mock_set_remaining(cost * (max_tries - 1)); assert!(matches!( try_find_program_address(&mut invoke_context, &[exceeded_seed], &address), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded), )); let exceeded_seeds: &[&[u8]] = &[ &[1], @@ -3962,9 +3933,7 @@ mod tests { invoke_context.mock_set_remaining(cost * (max_tries - 1)); assert!(matches!( try_find_program_address(&mut invoke_context, exceeded_seeds, &address), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded) - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::BadSeeds(PubkeyError::MaxSeedLengthExceeded), )); assert!(matches!( @@ -3975,9 +3944,7 @@ mod tests { true, SyscallTryFindProgramAddress::call, ), - Err(EbpfError::UserError(error)) if error.downcast_ref::().unwrap() == &BpfError::SyscallError( - SyscallError::CopyOverlapping - ), + Result::Err(error) if error.downcast_ref::().unwrap() == &SyscallError::CopyOverlapping, )); } diff --git a/programs/bpf_loader/src/syscalls/sysvar.rs b/programs/bpf_loader/src/syscalls/sysvar.rs index 7cf4d440da8d77..e1d5529873adbf 100644 --- a/programs/bpf_loader/src/syscalls/sysvar.rs +++ b/programs/bpf_loader/src/syscalls/sysvar.rs @@ -6,7 +6,7 @@ fn get_sysvar( check_aligned: bool, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, -) -> Result { +) -> Result> { consume_compute_meter( invoke_context, invoke_context @@ -33,7 +33,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { get_sysvar( invoke_context.get_sysvar_cache().get_clock(), var_addr, @@ -55,7 +55,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { get_sysvar( invoke_context.get_sysvar_cache().get_epoch_schedule(), var_addr, @@ -77,7 +77,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { #[allow(deprecated)] { get_sysvar( @@ -102,7 +102,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result { + ) -> Result> { get_sysvar( invoke_context.get_sysvar_cache().get_rent(), var_addr, diff --git a/programs/loader-v3/src/lib.rs b/programs/loader-v3/src/lib.rs index 0cbdeee3f349d9..68022734397209 100644 --- a/programs/loader-v3/src/lib.rs +++ b/programs/loader-v3/src/lib.rs @@ -215,10 +215,10 @@ fn execute( match result { ProgramResult::Ok(status) if status != SUCCESS => { - let error = status.into(); - Err(error) + let error: InstructionError = status.into(); + Err(Box::new(error) as Box) } - ProgramResult::Err(_) => Err(Box::new(InstructionError::ProgramFailedToComplete)), + ProgramResult::Err(error) => Err(error), _ => Ok(()), } } From 56aa5d7784ef44714b1c3e9211b0cdb88ac4ba74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 13 Mar 2023 17:42:18 +0100 Subject: [PATCH 6/8] Removes BpfError. --- programs/bpf_loader/src/lib.rs | 15 +-------------- programs/bpf_loader/src/syscalls/mod.rs | 7 ------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index cf3455fd50ba9d..f0d572c723d6bc 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -26,9 +26,8 @@ use { aligned_memory::AlignedMemory, ebpf::{self, HOST_ALIGN, MM_HEAP_START}, elf::Executable, - error::UserDefinedError, memory_region::{MemoryCowCallback, MemoryMapping, MemoryRegion}, - verifier::{RequisiteVerifier, VerifierError}, + verifier::RequisiteVerifier, vm::{ContextObject, EbpfVm, ProgramResult, VerifiedExecutable}, }, solana_sdk::{ @@ -61,12 +60,10 @@ use { }, std::{ cell::{RefCell, RefMut}, - fmt::Debug, mem, rc::Rc, sync::{atomic::Ordering, Arc}, }, - thiserror::Error, }; solana_sdk::declare_builtin!( @@ -75,16 +72,6 @@ solana_sdk::declare_builtin!( solana_bpf_loader_program::process_instruction ); -/// Errors returned by functions the BPF Loader registers with the VM -#[derive(Debug, Error, PartialEq, Eq)] -pub enum BpfError { - #[error("{0}")] - VerifierError(#[from] VerifierError), - #[error("{0}")] - SyscallError(#[from] SyscallError), -} -impl UserDefinedError for BpfError {} - #[allow(clippy::too_many_arguments)] pub fn load_program_from_bytes( feature_set: &FeatureSet, diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index 66fe795c38c36f..d0e47110465527 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -11,13 +11,11 @@ pub use self::{ }; #[allow(deprecated)] use { - crate::BpfError, solana_program_runtime::{ compute_budget::ComputeBudget, ic_logger_msg, ic_msg, invoke_context::InvokeContext, stable_log, timings::ExecuteTimings, }, solana_rbpf::{ - error::EbpfError, memory_region::{AccessType, MemoryMapping}, vm::{BuiltInProgram, Config, ProgramResult, PROGRAM_ENVIRONMENT_KEY_SHIFT}, }, @@ -127,11 +125,6 @@ pub enum SyscallError { #[error("InvalidAttribute")] InvalidAttribute, } -impl From for EbpfError { - fn from(error: SyscallError) -> Self { - EbpfError::UserError(Box::::new(error.into())) - } -} fn consume_compute_meter( invoke_context: &InvokeContext, From d079811a8e5e0e9cd89ceca9f07100e69215bc4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 13 Mar 2023 14:18:33 +0100 Subject: [PATCH 7/8] Removes SyscallError::InstructionError. --- program-runtime/src/invoke_context.rs | 4 +- programs/bpf_loader/src/lib.rs | 21 ++-- programs/bpf_loader/src/syscalls/cpi.rs | 131 ++++++++------------- programs/bpf_loader/src/syscalls/mod.rs | 26 ++-- programs/bpf_loader/src/syscalls/sysvar.rs | 2 +- 5 files changed, 67 insertions(+), 117 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 7a3b7d43fce823..25c172c20b8281 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -758,13 +758,13 @@ impl<'a> InvokeContext<'a> { } /// Consume compute units - pub fn consume_checked(&self, amount: u64) -> Result<(), InstructionError> { + pub fn consume_checked(&self, amount: u64) -> Result<(), Box> { self.log_consumed_bpf_units(amount); let mut compute_meter = self.compute_meter.borrow_mut(); let exceeded = *compute_meter < amount; *compute_meter = compute_meter.saturating_sub(amount); if exceeded { - return Err(InstructionError::ComputationalBudgetExceeded); + return Err(Box::new(InstructionError::ComputationalBudgetExceeded)); } Ok(()) } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index f0d572c723d6bc..95ecc95c236877 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -10,7 +10,7 @@ pub mod upgradeable_with_jit; pub mod with_jit; use { - crate::{allocator_bump::BpfAllocator, syscalls::SyscallError}, + crate::allocator_bump::BpfAllocator, solana_measure::measure::Measure, solana_program_runtime::{ compute_budget::ComputeBudget, @@ -319,7 +319,7 @@ pub fn create_ebpf_vm<'a, 'b>( round_up_heap_size, )); if round_up_heap_size { - heap_cost_result.map_err(SyscallError::InstructionError)?; + heap_cost_result?; } let check_aligned = bpf_loader_deprecated::id() != invoke_context @@ -329,20 +329,17 @@ pub fn create_ebpf_vm<'a, 'b>( instruction_context .try_borrow_last_program_account(invoke_context.transaction_context) }) - .map(|program_account| *program_account.get_owner()) - .map_err(SyscallError::InstructionError)?; + .map(|program_account| *program_account.get_owner())?; let check_size = invoke_context .feature_set .is_active(&check_slice_translation_size::id()); let allocator = Rc::new(RefCell::new(BpfAllocator::new(heap, MM_HEAP_START))); - invoke_context - .set_syscall_context( - check_aligned, - check_size, - orig_account_lengths, - allocator.clone(), - ) - .map_err(SyscallError::InstructionError)?; + invoke_context.set_syscall_context( + check_aligned, + check_size, + orig_account_lengths, + allocator.clone(), + )?; let stack_len = stack.len(); let memory_mapping = create_memory_mapping( program.get_executable(), diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 5c295f75a79cd1..c56810f0a8ecd0 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -350,7 +350,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { invoke_context.get_check_size(), )?; if signers_seeds.len() > MAX_SIGNERS { - return Err(SyscallError::TooManySigners.into()); + return Err(Box::new(SyscallError::TooManySigners)); } for signer_seeds in signers_seeds.iter() { let untranslated_seeds = translate_slice::<&[u8]>( @@ -361,10 +361,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { invoke_context.get_check_size(), )?; if untranslated_seeds.len() > MAX_SEEDS { - return Err(SyscallError::InstructionError( - InstructionError::MaxSeedLengthExceeded, - ) - .into()); + return Err(Box::new(InstructionError::MaxSeedLengthExceeded)); } let seeds = untranslated_seeds .iter() @@ -583,7 +580,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { invoke_context.get_check_size(), )?; if signers_seeds.len() > MAX_SIGNERS { - return Err(SyscallError::TooManySigners.into()); + return Err(Box::new(SyscallError::TooManySigners)); } Ok(signers_seeds .iter() @@ -596,10 +593,8 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { invoke_context.get_check_size(), )?; if seeds.len() > MAX_SEEDS { - return Err(SyscallError::InstructionError( - InstructionError::MaxSeedLengthExceeded, - ) - .into()); + return Err(Box::new(InstructionError::MaxSeedLengthExceeded) + as Box); } let seeds_bytes = seeds .iter() @@ -613,8 +608,9 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { ) }) .collect::, Box>>()?; - Pubkey::create_program_address(&seeds_bytes, program_id) - .map_err(|err| SyscallError::BadSeeds(err).into()) + Pubkey::create_program_address(&seeds_bytes, program_id).map_err(|err| { + Box::new(SyscallError::BadSeeds(err)) as Box + }) }) .collect::, Box>>()?) } else { @@ -677,16 +673,12 @@ where ) -> Result, Box>, { let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context - .get_current_instruction_context() - .map_err(SyscallError::InstructionError)?; + let instruction_context = transaction_context.get_current_instruction_context()?; let mut accounts = Vec::with_capacity(instruction_accounts.len().saturating_add(1)); let program_account_index = program_indices .last() - .ok_or(SyscallError::InstructionError( - InstructionError::MissingAccount, - ))?; + .ok_or_else(|| Box::new(InstructionError::MissingAccount))?; accounts.push((*program_account_index, None)); // unwrapping here is fine: we're in a syscall and the method below fails @@ -699,16 +691,13 @@ where continue; // Skip duplicate account } - let callee_account = instruction_context - .try_borrow_instruction_account( - transaction_context, - instruction_account.index_in_caller, - ) - .map_err(SyscallError::InstructionError)?; + let callee_account = instruction_context.try_borrow_instruction_account( + transaction_context, + instruction_account.index_in_caller, + )?; let account_key = invoke_context .transaction_context - .get_key_of_account_at_index(instruction_account.index_in_transaction) - .map_err(SyscallError::InstructionError)?; + .get_key_of_account_at_index(instruction_account.index_in_transaction)?; if callee_account.is_executable() { // Use the known account @@ -730,7 +719,7 @@ where "Internal error: index mismatch for account {}", account_key ); - SyscallError::InstructionError(InstructionError::MissingAccount) + Box::new(InstructionError::MissingAccount) })?; // build the CallerAccount corresponding to this account. @@ -765,7 +754,7 @@ where "Instruction references an unknown account {}", account_key ); - return Err(SyscallError::InstructionError(InstructionError::MissingAccount).into()); + return Err(Box::new(InstructionError::MissingAccount)); } } @@ -784,21 +773,19 @@ fn check_instruction_size( let data_len = data_len as u64; let max_data_len = MAX_CPI_INSTRUCTION_DATA_LEN; if data_len > max_data_len { - return Err(SyscallError::MaxInstructionDataLenExceeded { + return Err(Box::new(SyscallError::MaxInstructionDataLenExceeded { data_len, max_data_len, - } - .into()); + })); } let num_accounts = num_accounts as u64; let max_accounts = MAX_CPI_INSTRUCTION_ACCOUNTS as u64; if num_accounts > max_accounts { - return Err(SyscallError::MaxInstructionAccountsExceeded { + return Err(Box::new(SyscallError::MaxInstructionAccountsExceeded { num_accounts, max_accounts, - } - .into()); + })); } } else { let max_size = invoke_context.get_compute_budget().max_cpi_instruction_size; @@ -806,7 +793,7 @@ fn check_instruction_size( .saturating_mul(size_of::()) .saturating_add(data_len); if size > max_size { - return Err(SyscallError::InstructionTooLarge(size, max_size).into()); + return Err(Box::new(SyscallError::InstructionTooLarge(size, max_size))); } } Ok(()) @@ -831,11 +818,10 @@ fn check_account_infos( let num_account_infos = num_account_infos as u64; let max_account_infos = max_cpi_account_infos as u64; if num_account_infos > max_account_infos { - return Err(SyscallError::MaxInstructionAccountInfosExceeded { + return Err(Box::new(SyscallError::MaxInstructionAccountInfosExceeded { num_account_infos, max_account_infos, - } - .into()); + })); } } else { let adjusted_len = num_account_infos.saturating_mul(size_of::()); @@ -843,7 +829,7 @@ fn check_account_infos( if adjusted_len > invoke_context.get_compute_budget().max_cpi_instruction_size { // Cap the number of account_infos a caller can pass to approximate // maximum that accounts that could be passed in an instruction - return Err(SyscallError::TooManyAccounts.into()); + return Err(Box::new(SyscallError::TooManyAccounts)); }; } Ok(()) @@ -871,7 +857,7 @@ fn check_authorized_program( invoke_context.feature_set.is_active(feature_id) }) { - return Err(SyscallError::ProgramNotSupported(*program_id).into()); + return Err(Box::new(SyscallError::ProgramNotSupported(*program_id))); } Ok(()) } @@ -897,12 +883,8 @@ fn cpi_common( let instruction = S::translate_instruction(instruction_addr, memory_mapping, invoke_context)?; let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context - .get_current_instruction_context() - .map_err(SyscallError::InstructionError)?; - let caller_program_id = instruction_context - .get_last_program_key(transaction_context) - .map_err(SyscallError::InstructionError)?; + let instruction_context = transaction_context.get_current_instruction_context()?; + let caller_program_id = instruction_context.get_last_program_key(transaction_context)?; let signers = S::translate_signers( caller_program_id, signers_seeds_addr, @@ -910,9 +892,8 @@ fn cpi_common( memory_mapping, invoke_context, )?; - let (instruction_accounts, program_indices) = invoke_context - .prepare_instruction(&instruction, &signers) - .map_err(SyscallError::InstructionError)?; + let (instruction_accounts, program_indices) = + invoke_context.prepare_instruction(&instruction, &signers)?; check_authorized_program(&instruction.program_id, &instruction.data, invoke_context)?; let mut accounts = S::translate_accounts( &instruction_accounts, @@ -925,21 +906,17 @@ fn cpi_common( // Process the callee instruction let mut compute_units_consumed = 0; - invoke_context - .process_instruction( - &instruction.data, - &instruction_accounts, - &program_indices, - &mut compute_units_consumed, - &mut ExecuteTimings::default(), - ) - .map_err(SyscallError::InstructionError)?; + invoke_context.process_instruction( + &instruction.data, + &instruction_accounts, + &program_indices, + &mut compute_units_consumed, + &mut ExecuteTimings::default(), + )?; // re-bind to please the borrow checker let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context - .get_current_instruction_context() - .map_err(SyscallError::InstructionError)?; + let instruction_context = transaction_context.get_current_instruction_context()?; // CPI exit. // @@ -947,8 +924,7 @@ fn cpi_common( for (index_in_caller, caller_account) in accounts.iter_mut() { if let Some(caller_account) = caller_account { let callee_account = instruction_context - .try_borrow_instruction_account(transaction_context, *index_in_caller) - .map_err(SyscallError::InstructionError)?; + .try_borrow_instruction_account(transaction_context, *index_in_caller)?; update_caller_account( invoke_context, memory_mapping, @@ -979,9 +955,7 @@ fn update_callee_account( .is_active(&disable_cpi_setting_executable_and_rent_epoch::id()); if callee_account.get_lamports() != *caller_account.lamports { - callee_account - .set_lamports(*caller_account.lamports) - .map_err(SyscallError::InstructionError)?; + callee_account.set_lamports(*caller_account.lamports)?; } // The redundant check helps to avoid the expensive data comparison if we can @@ -989,9 +963,7 @@ fn update_callee_account( .can_data_be_resized(caller_account.data.len()) .and_then(|_| callee_account.can_data_be_changed()) { - Ok(()) => callee_account - .set_data_from_slice(caller_account.data) - .map_err(SyscallError::InstructionError)?, + Ok(()) => callee_account.set_data_from_slice(caller_account.data)?, Err(err) if callee_account.get_data() != caller_account.data => { return Err(Box::new(err)); } @@ -1001,16 +973,12 @@ fn update_callee_account( if !is_disable_cpi_setting_executable_and_rent_epoch_active && callee_account.is_executable() != caller_account.executable { - callee_account - .set_executable(caller_account.executable) - .map_err(SyscallError::InstructionError)?; + callee_account.set_executable(caller_account.executable)?; } // Change the owner at the end so that we are allowed to change the lamports and data before if callee_account.get_owner() != caller_account.owner { - callee_account - .set_owner(caller_account.owner.as_ref()) - .map_err(SyscallError::InstructionError)?; + callee_account.set_owner(caller_account.owner.as_ref())?; } // BorrowedAccount doesn't allow changing the rent epoch. Drop it and use @@ -1019,8 +987,7 @@ fn update_callee_account( drop(callee_account); let callee_account = invoke_context .transaction_context - .get_account_at_index(index_in_transaction) - .map_err(SyscallError::InstructionError)?; + .get_account_at_index(index_in_transaction)?; if !is_disable_cpi_setting_executable_and_rent_epoch_active && callee_account.borrow().rent_epoch() != caller_account.rent_epoch { @@ -1028,7 +995,7 @@ fn update_callee_account( .feature_set .is_active(&enable_early_verification_of_account_modifications::id()) { - return Err(SyscallError::InstructionError(InstructionError::RentEpochModified).into()); + return Err(Box::new(InstructionError::RentEpochModified)); } else { callee_account .borrow_mut() @@ -1067,15 +1034,13 @@ fn update_caller_account( "Account data size realloc limited to {} in inner instructions", MAX_PERMITTED_DATA_INCREASE ); - return Err(SyscallError::InstructionError(InstructionError::InvalidRealloc).into()); + return Err(Box::new(InstructionError::InvalidRealloc)); } if new_len < caller_account.data.len() { caller_account .data .get_mut(new_len..) - .ok_or(SyscallError::InstructionError( - InstructionError::AccountDataTooSmall, - ))? + .ok_or_else(|| Box::new(InstructionError::AccountDataTooSmall))? .fill(0); } caller_account.data = translate_slice_mut::( @@ -1113,7 +1078,7 @@ fn update_caller_account( .get(0..new_len) .ok_or(SyscallError::InvalidLength)?; if to_slice.len() != from_slice.len() { - return Err(SyscallError::InstructionError(InstructionError::AccountDataTooSmall).into()); + return Err(Box::new(InstructionError::AccountDataTooSmall)); } to_slice.copy_from_slice(from_slice); diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index d0e47110465527..e2a0cb21b05745 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -92,8 +92,6 @@ pub enum SyscallError { BadSeeds(PubkeyError), #[error("Program {0} not supported by inner instructions")] ProgramNotSupported(Pubkey), - #[error("{0}")] - InstructionError(InstructionError), #[error("Unaligned pointer")] UnalignedPointer, #[error("Too many signers")] @@ -130,9 +128,7 @@ fn consume_compute_meter( invoke_context: &InvokeContext, amount: u64, ) -> Result<(), Box> { - invoke_context - .consume_checked(amount) - .map_err(SyscallError::InstructionError)?; + invoke_context.consume_checked(amount)?; Ok(()) } @@ -535,9 +531,7 @@ declare_syscall!( _arg5: u64, _memory_mapping: &mut MemoryMapping, ) -> Result> { - let allocator = invoke_context - .get_allocator() - .map_err(SyscallError::InstructionError)?; + let allocator = invoke_context.get_allocator()?; let mut allocator = allocator .try_borrow_mut() .map_err(|_| SyscallError::InvokeContextBorrowFailed)?; @@ -1398,12 +1392,9 @@ declare_syscall!( .get_current_instruction_context() .and_then(|instruction_context| { instruction_context.get_last_program_key(transaction_context) - }) - .map_err(SyscallError::InstructionError)?; + })?; - transaction_context - .set_return_data(program_id, return_data) - .map_err(SyscallError::InstructionError)?; + transaction_context.set_return_data(program_id, return_data)?; Ok(0) } @@ -1506,8 +1497,7 @@ declare_syscall!( for index_in_trace in (0..instruction_trace_length).rev() { let instruction_context = invoke_context .transaction_context - .get_instruction_context_at_index_in_trace(index_in_trace) - .map_err(SyscallError::InstructionError)?; + .get_instruction_context_at_index_in_trace(index_in_trace)?; if (stop_sibling_instruction_search_at_parent || instruction_context.get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT) && instruction_context.get_stack_height() < stack_height @@ -1595,8 +1585,7 @@ declare_syscall!( } *program_id = *instruction_context - .get_last_program_key(invoke_context.transaction_context) - .map_err(SyscallError::InstructionError)?; + .get_last_program_key(invoke_context.transaction_context)?; data.clone_from_slice(instruction_context.get_instruction_data()); let account_metas = (0..instruction_context.get_number_of_instruction_accounts()) .map(|instruction_account_index| { @@ -1615,8 +1604,7 @@ declare_syscall!( .is_instruction_account_writable(instruction_account_index)?, }) }) - .collect::, InstructionError>>() - .map_err(SyscallError::InstructionError)?; + .collect::, InstructionError>>()?; accounts.clone_from_slice(account_metas.as_slice()); } result_header.data_len = instruction_context.get_instruction_data().len() as u64; diff --git a/programs/bpf_loader/src/syscalls/sysvar.rs b/programs/bpf_loader/src/syscalls/sysvar.rs index e1d5529873adbf..fbd8624fc024c3 100644 --- a/programs/bpf_loader/src/syscalls/sysvar.rs +++ b/programs/bpf_loader/src/syscalls/sysvar.rs @@ -16,7 +16,7 @@ fn get_sysvar( )?; let var = translate_type_mut::(memory_mapping, var_addr, check_aligned)?; - let sysvar: Arc = sysvar.map_err(SyscallError::InstructionError)?; + let sysvar: Arc = sysvar?; *var = T::clone(sysvar.as_ref()); Ok(SUCCESS) From 778c71dfa4b93332d119689541facc638bec1ea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 5 Apr 2023 08:41:05 +0200 Subject: [PATCH 8/8] Adds a type alias for Box in syscalls. --- programs/bpf_loader/src/syscalls/cpi.rs | 68 +++++++++---------- programs/bpf_loader/src/syscalls/logging.rs | 10 +-- programs/bpf_loader/src/syscalls/mem_ops.rs | 13 ++-- programs/bpf_loader/src/syscalls/mod.rs | 75 ++++++++++----------- programs/bpf_loader/src/syscalls/sysvar.rs | 10 +-- 5 files changed, 82 insertions(+), 94 deletions(-) diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index c56810f0a8ecd0..e1b29fed3eae5c 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -39,7 +39,7 @@ impl<'a> CallerAccount<'a> { _vm_addr: u64, account_info: &AccountInfo, original_data_len: usize, - ) -> Result, Box> { + ) -> Result, Error> { // account_info points to host memory. The addresses used internally are // in vm space so they need to be translated. @@ -129,7 +129,7 @@ impl<'a> CallerAccount<'a> { vm_addr: u64, account_info: &SolAccountInfo, original_data_len: usize, - ) -> Result, Box> { + ) -> Result, Error> { // account_info points to host memory. The addresses used internally are // in vm space so they need to be translated. @@ -212,7 +212,7 @@ trait SyscallInvokeSigned { addr: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result>; + ) -> Result; fn translate_accounts<'a>( instruction_accounts: &[InstructionAccount], program_indices: &[IndexOfAccount], @@ -220,14 +220,14 @@ trait SyscallInvokeSigned { account_infos_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result, Box>; + ) -> Result, Error>; fn translate_signers( program_id: &Pubkey, signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &InvokeContext, - ) -> Result, Box>; + ) -> Result, Error>; } declare_syscall!( @@ -241,7 +241,7 @@ declare_syscall!( signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { cpi_common::( invoke_context, instruction_addr, @@ -259,7 +259,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { addr: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result> { + ) -> Result { let ix = translate_type::( memory_mapping, addr, @@ -312,7 +312,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { account_infos_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result, Box> { + ) -> Result, Error> { let (account_infos, account_info_keys) = translate_account_infos( account_infos_addr, account_infos_len, @@ -339,7 +339,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &InvokeContext, - ) -> Result, Box> { + ) -> Result, Error> { let mut signers = Vec::new(); if signers_seeds_len > 0 { let signers_seeds = translate_slice::<&[&[u8]]>( @@ -374,7 +374,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust { invoke_context.get_check_size(), ) }) - .collect::, Box>>()?; + .collect::, Error>>()?; let signer = Pubkey::create_program_address(&seeds, program_id) .map_err(SyscallError::BadSeeds)?; signers.push(signer); @@ -450,7 +450,7 @@ declare_syscall!( signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { cpi_common::( invoke_context, instruction_addr, @@ -468,7 +468,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { addr: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result> { + ) -> Result { let ix_c = translate_type::( memory_mapping, addr, @@ -527,7 +527,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { is_writable: meta_c.is_writable, }) }) - .collect::, Box>>()?; + .collect::, Error>>()?; Ok(StableInstruction { accounts: accounts.into(), @@ -543,7 +543,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { account_infos_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, - ) -> Result, Box> { + ) -> Result, Error> { let (account_infos, account_info_keys) = translate_account_infos( account_infos_addr, account_infos_len, @@ -570,7 +570,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, invoke_context: &InvokeContext, - ) -> Result, Box> { + ) -> Result, Error> { if signers_seeds_len > 0 { let signers_seeds = translate_slice::( memory_mapping, @@ -593,8 +593,7 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { invoke_context.get_check_size(), )?; if seeds.len() > MAX_SEEDS { - return Err(Box::new(InstructionError::MaxSeedLengthExceeded) - as Box); + return Err(Box::new(InstructionError::MaxSeedLengthExceeded) as Error); } let seeds_bytes = seeds .iter() @@ -607,12 +606,11 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC { invoke_context.get_check_size(), ) }) - .collect::, Box>>()?; - Pubkey::create_program_address(&seeds_bytes, program_id).map_err(|err| { - Box::new(SyscallError::BadSeeds(err)) as Box - }) + .collect::, Error>>()?; + Pubkey::create_program_address(&seeds_bytes, program_id) + .map_err(|err| Box::new(SyscallError::BadSeeds(err)) as Error) }) - .collect::, Box>>()?) + .collect::, Error>>()?) } else { Ok(vec![]) } @@ -625,7 +623,7 @@ fn translate_account_infos<'a, T, F>( key_addr: F, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, -) -> Result<(&'a [T], Vec<&'a Pubkey>), Box> +) -> Result<(&'a [T], Vec<&'a Pubkey>), Error> where F: Fn(&T) -> u64, { @@ -646,7 +644,7 @@ where invoke_context.get_check_aligned(), ) }) - .collect::, Box>>()?; + .collect::, Error>>()?; Ok((account_infos, account_info_keys)) } @@ -662,15 +660,9 @@ fn translate_and_update_accounts<'a, T, F>( invoke_context: &mut InvokeContext, memory_mapping: &MemoryMapping, do_translate: F, -) -> Result, Box> +) -> Result, Error> where - F: Fn( - &InvokeContext, - &MemoryMapping, - u64, - &T, - usize, - ) -> Result, Box>, + F: Fn(&InvokeContext, &MemoryMapping, u64, &T, usize) -> Result, Error>, { let transaction_context = &invoke_context.transaction_context; let instruction_context = transaction_context.get_current_instruction_context()?; @@ -765,7 +757,7 @@ fn check_instruction_size( num_accounts: usize, data_len: usize, invoke_context: &mut InvokeContext, -) -> Result<(), Box> { +) -> Result<(), Error> { if invoke_context .feature_set .is_active(&feature_set::loosen_cpi_size_restriction::id()) @@ -802,7 +794,7 @@ fn check_instruction_size( fn check_account_infos( num_account_infos: usize, invoke_context: &mut InvokeContext, -) -> Result<(), Box> { +) -> Result<(), Error> { if invoke_context .feature_set .is_active(&feature_set::loosen_cpi_size_restriction::id()) @@ -839,7 +831,7 @@ fn check_authorized_program( program_id: &Pubkey, instruction_data: &[u8], invoke_context: &InvokeContext, -) -> Result<(), Box> { +) -> Result<(), Error> { if native_loader::check_id(program_id) || bpf_loader::check_id(program_id) || bpf_loader_deprecated::check_id(program_id) @@ -871,7 +863,7 @@ fn cpi_common( signers_seeds_addr: u64, signers_seeds_len: u64, memory_mapping: &mut MemoryMapping, -) -> Result> { +) -> Result { // CPI entry. // // Translate the inputs to the syscall and synchronize the caller's account @@ -949,7 +941,7 @@ fn update_callee_account( invoke_context: &InvokeContext, caller_account: &CallerAccount, mut callee_account: BorrowedAccount<'_>, -) -> Result<(), Box> { +) -> Result<(), Error> { let is_disable_cpi_setting_executable_and_rent_epoch_active = invoke_context .feature_set .is_active(&disable_cpi_setting_executable_and_rent_epoch::id()); @@ -1019,7 +1011,7 @@ fn update_caller_account( memory_mapping: &MemoryMapping, caller_account: &mut CallerAccount, callee_account: &BorrowedAccount<'_>, -) -> Result<(), Box> { +) -> Result<(), Error> { *caller_account.lamports = callee_account.get_lamports(); *caller_account.owner = *callee_account.get_owner(); let new_len = callee_account.get_data().len(); diff --git a/programs/bpf_loader/src/syscalls/logging.rs b/programs/bpf_loader/src/syscalls/logging.rs index 48dd7f670c21b7..f6d69153d2bc52 100644 --- a/programs/bpf_loader/src/syscalls/logging.rs +++ b/programs/bpf_loader/src/syscalls/logging.rs @@ -11,7 +11,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context .get_compute_budget() .syscall_base_cost @@ -47,7 +47,7 @@ declare_syscall!( arg4: u64, arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context.get_compute_budget().log_64_units; consume_compute_meter(invoke_context, cost)?; @@ -70,7 +70,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context.get_compute_budget().syscall_base_cost; consume_compute_meter(invoke_context, cost)?; @@ -94,7 +94,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context.get_compute_budget().log_pubkey_units; consume_compute_meter(invoke_context, cost)?; @@ -119,7 +119,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; diff --git a/programs/bpf_loader/src/syscalls/mem_ops.rs b/programs/bpf_loader/src/syscalls/mem_ops.rs index 5460ccc158bfab..d501311461339f 100644 --- a/programs/bpf_loader/src/syscalls/mem_ops.rs +++ b/programs/bpf_loader/src/syscalls/mem_ops.rs @@ -1,9 +1,6 @@ use {super::*, crate::declare_syscall}; -fn mem_op_consume( - invoke_context: &mut InvokeContext, - n: u64, -) -> Result<(), Box> { +fn mem_op_consume(invoke_context: &mut InvokeContext, n: u64) -> Result<(), Error> { let compute_budget = invoke_context.get_compute_budget(); let cost = compute_budget .mem_op_base_cost @@ -22,7 +19,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { mem_op_consume(invoke_context, n)?; if !is_nonoverlapping(src_addr, n, dst_addr, n) { @@ -69,7 +66,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { mem_op_consume(invoke_context, n)?; let dst = translate_slice_mut::( @@ -104,7 +101,7 @@ declare_syscall!( cmp_result_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { mem_op_consume(invoke_context, n)?; let s1 = translate_slice::( @@ -152,7 +149,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { mem_op_consume(invoke_context, n)?; let s = translate_slice_mut::( diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index e2a0cb21b05745..bc0f41f324e208 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -124,10 +124,9 @@ pub enum SyscallError { InvalidAttribute, } -fn consume_compute_meter( - invoke_context: &InvokeContext, - amount: u64, -) -> Result<(), Box> { +type Error = Box; + +fn consume_compute_meter(invoke_context: &InvokeContext, amount: u64) -> Result<(), Error> { invoke_context.consume_checked(amount)?; Ok(()) } @@ -148,7 +147,7 @@ pub fn create_loader<'a>( reject_deployment_of_broken_elfs: bool, disable_deploy_of_alloc_free_syscall: bool, debugging_features: bool, -) -> Result>>, Box> { +) -> Result>>, Error> { use rand::Rng; let config = Config { max_call_depth: compute_budget.max_call_depth, @@ -322,7 +321,7 @@ fn translate( access_type: AccessType, vm_addr: u64, len: u64, -) -> Result> { +) -> Result { memory_mapping.map(access_type, vm_addr, len, 0).into() } @@ -331,7 +330,7 @@ fn translate_type_inner<'a, T>( access_type: AccessType, vm_addr: u64, check_aligned: bool, -) -> Result<&'a mut T, Box> { +) -> Result<&'a mut T, Error> { let host_addr = translate(memory_mapping, access_type, vm_addr, size_of::() as u64)?; if check_aligned && (host_addr as *mut T as usize).wrapping_rem(align_of::()) != 0 { @@ -343,14 +342,14 @@ fn translate_type_mut<'a, T>( memory_mapping: &MemoryMapping, vm_addr: u64, check_aligned: bool, -) -> Result<&'a mut T, Box> { +) -> Result<&'a mut T, Error> { translate_type_inner::(memory_mapping, AccessType::Store, vm_addr, check_aligned) } fn translate_type<'a, T>( memory_mapping: &MemoryMapping, vm_addr: u64, check_aligned: bool, -) -> Result<&'a T, Box> { +) -> Result<&'a T, Error> { translate_type_inner::(memory_mapping, AccessType::Load, vm_addr, check_aligned) .map(|value| &*value) } @@ -362,7 +361,7 @@ fn translate_slice_inner<'a, T>( len: u64, check_aligned: bool, check_size: bool, -) -> Result<&'a mut [T], Box> { +) -> Result<&'a mut [T], Error> { if len == 0 { return Ok(&mut []); } @@ -385,7 +384,7 @@ fn translate_slice_mut<'a, T>( len: u64, check_aligned: bool, check_size: bool, -) -> Result<&'a mut [T], Box> { +) -> Result<&'a mut [T], Error> { translate_slice_inner::( memory_mapping, AccessType::Store, @@ -401,7 +400,7 @@ fn translate_slice<'a, T>( len: u64, check_aligned: bool, check_size: bool, -) -> Result<&'a [T], Box> { +) -> Result<&'a [T], Error> { translate_slice_inner::( memory_mapping, AccessType::Load, @@ -422,8 +421,8 @@ fn translate_string_and_do( check_aligned: bool, check_size: bool, stop_truncating_strings_in_syscalls: bool, - work: &mut dyn FnMut(&str) -> Result>, -) -> Result> { + work: &mut dyn FnMut(&str) -> Result, +) -> Result { let buf = translate_slice::(memory_mapping, addr, len, check_aligned, check_size)?; let msg = if stop_truncating_strings_in_syscalls { buf @@ -480,7 +479,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { Err(SyscallError::Abort.into()) } ); @@ -497,7 +496,7 @@ declare_syscall!( column: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { consume_compute_meter(invoke_context, len)?; translate_string_and_do( @@ -530,7 +529,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let allocator = invoke_context.get_allocator()?; let mut allocator = allocator .try_borrow_mut() @@ -566,7 +565,7 @@ fn translate_and_check_program_address_inputs<'a>( memory_mapping: &mut MemoryMapping, check_aligned: bool, check_size: bool, -) -> Result<(Vec<&'a [u8]>, &'a Pubkey), Box> { +) -> Result<(Vec<&'a [u8]>, &'a Pubkey), Error> { let untranslated_seeds = translate_slice::<&[&u8]>( memory_mapping, seeds_addr, @@ -591,7 +590,7 @@ fn translate_and_check_program_address_inputs<'a>( check_size, ) }) - .collect::, Box>>()?; + .collect::, Error>>()?; let program_id = translate_type::(memory_mapping, program_id_addr, check_aligned)?; Ok((seeds, program_id)) } @@ -607,7 +606,7 @@ declare_syscall!( address_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context .get_compute_budget() .create_program_address_units; @@ -651,7 +650,7 @@ declare_syscall!( address_addr: u64, bump_seed_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context .get_compute_budget() .create_program_address_units; @@ -721,7 +720,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let compute_budget = invoke_context.get_compute_budget(); if compute_budget.sha256_max_slices < vals_len { ic_msg!( @@ -784,7 +783,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let compute_budget = invoke_context.get_compute_budget(); if compute_budget.sha256_max_slices < vals_len { ic_msg!( @@ -847,7 +846,7 @@ declare_syscall!( result_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let cost = invoke_context.get_compute_budget().secp256k1_recover_cost; consume_compute_meter(invoke_context, cost)?; @@ -939,7 +938,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { use solana_zk_token_sdk::curve25519::{curve_syscall_traits::*, edwards, ristretto}; match curve_id { CURVE25519_EDWARDS => { @@ -996,7 +995,7 @@ declare_syscall!( right_input_addr: u64, result_point_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { use solana_zk_token_sdk::curve25519::{ curve_syscall_traits::*, edwards, ristretto, scalar, }; @@ -1197,7 +1196,7 @@ declare_syscall!( points_len: u64, result_point_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { use solana_zk_token_sdk::curve25519::{ curve_syscall_traits::*, edwards, ristretto, scalar, }; @@ -1300,7 +1299,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let compute_budget = invoke_context.get_compute_budget(); if compute_budget.sha256_max_slices < vals_len { ic_msg!( @@ -1363,7 +1362,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let budget = invoke_context.get_compute_budget(); let cost = len @@ -1411,7 +1410,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; @@ -1478,7 +1477,7 @@ declare_syscall!( data_addr: u64, accounts_addr: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; @@ -1627,7 +1626,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, _memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let budget = invoke_context.get_compute_budget(); consume_compute_meter(invoke_context, budget.syscall_base_cost)?; @@ -1647,7 +1646,7 @@ declare_syscall!( result_addr: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { use solana_sdk::alt_bn128::prelude::{ALT_BN128_ADD, ALT_BN128_MUL, ALT_BN128_PAIRING}; let budget = invoke_context.get_compute_budget(); let (cost, output): (u64, usize) = match group_op { @@ -1732,7 +1731,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { let params = &translate_slice::( memory_mapping, params, @@ -3447,7 +3446,7 @@ mod tests { program_id: &Pubkey, overlap_outputs: bool, syscall: BuiltInFunction>, - ) -> Result<(Pubkey, u8), Box> { + ) -> Result<(Pubkey, u8), Error> { const SEEDS_VA: u64 = 0x100000000; const PROGRAM_ID_VA: u64 = 0x200000000; const ADDRESS_VA: u64 = 0x300000000; @@ -3494,14 +3493,14 @@ mod tests { &mut memory_mapping, &mut result, ); - Result::>::from(result).map(|_| (address, bump_seed)) + Result::::from(result).map(|_| (address, bump_seed)) } fn create_program_address( invoke_context: &mut InvokeContext, seeds: &[&[u8]], address: &Pubkey, - ) -> Result> { + ) -> Result { let (address, _) = call_program_address_common( invoke_context, seeds, @@ -3516,7 +3515,7 @@ mod tests { invoke_context: &mut InvokeContext, seeds: &[&[u8]], address: &Pubkey, - ) -> Result<(Pubkey, u8), Box> { + ) -> Result<(Pubkey, u8), Error> { call_program_address_common( invoke_context, seeds, diff --git a/programs/bpf_loader/src/syscalls/sysvar.rs b/programs/bpf_loader/src/syscalls/sysvar.rs index fbd8624fc024c3..9ab061b8a25ba7 100644 --- a/programs/bpf_loader/src/syscalls/sysvar.rs +++ b/programs/bpf_loader/src/syscalls/sysvar.rs @@ -6,7 +6,7 @@ fn get_sysvar( check_aligned: bool, memory_mapping: &mut MemoryMapping, invoke_context: &mut InvokeContext, -) -> Result> { +) -> Result { consume_compute_meter( invoke_context, invoke_context @@ -33,7 +33,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { get_sysvar( invoke_context.get_sysvar_cache().get_clock(), var_addr, @@ -55,7 +55,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { get_sysvar( invoke_context.get_sysvar_cache().get_epoch_schedule(), var_addr, @@ -77,7 +77,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { #[allow(deprecated)] { get_sysvar( @@ -102,7 +102,7 @@ declare_syscall!( _arg4: u64, _arg5: u64, memory_mapping: &mut MemoryMapping, - ) -> Result> { + ) -> Result { get_sysvar( invoke_context.get_sysvar_cache().get_rent(), var_addr,