From 67d9faaefc38796f90491cdc64fcd68003df7cc3 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2020 23:12:01 +0000 Subject: [PATCH] Bump compute budget (#11864) (#11867) * Bump compute budget * nudge (cherry picked from commit ea179ad762c5a56085de65b3d50aa31cd45ff5d5) Co-authored-by: Jack May --- programs/bpf/benches/bpf_loader.rs | 5 +- programs/bpf_loader/src/bpf_verifier.rs | 2 +- programs/bpf_loader/src/lib.rs | 31 ++++++++++++- programs/bpf_loader/src/syscalls.rs | 61 +++++++++++++++++-------- runtime/src/bank.rs | 37 ++++++++++----- runtime/src/message_processor.rs | 46 +++++++------------ sdk/src/entrypoint_native.rs | 33 +++++++++++++ 7 files changed, 150 insertions(+), 65 deletions(-) diff --git a/programs/bpf/benches/bpf_loader.rs b/programs/bpf/benches/bpf_loader.rs index d03ac61bc4..af0780edad 100644 --- a/programs/bpf/benches/bpf_loader.rs +++ b/programs/bpf/benches/bpf_loader.rs @@ -7,7 +7,7 @@ use solana_rbpf::EbpfVm; use solana_sdk::{ account::Account, bpf_loader, - entrypoint_native::{ComputeMeter, InvokeContext, Logger, ProcessInstruction}, + entrypoint_native::{ComputeBudget, ComputeMeter, InvokeContext, Logger, ProcessInstruction}, instruction::{CompiledInstruction, InstructionError}, message::Message, pubkey::Pubkey, @@ -166,6 +166,9 @@ impl InvokeContext for MockInvokeContext { fn is_cross_program_supported(&self) -> bool { true } + fn get_compute_budget(&self) -> ComputeBudget { + ComputeBudget::default() + } fn get_compute_meter(&self) -> Rc> { Rc::new(RefCell::new(self.mock_compute_meter.clone())) } diff --git a/programs/bpf_loader/src/bpf_verifier.rs b/programs/bpf_loader/src/bpf_verifier.rs index eba9e602b5..a3261b2844 100644 --- a/programs/bpf_loader/src/bpf_verifier.rs +++ b/programs/bpf_loader/src/bpf_verifier.rs @@ -3,7 +3,7 @@ use solana_rbpf::ebpf; use thiserror::Error; /// Error definitions -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq)] pub enum VerifierError { /// ProgramLengthNotMultiple #[error("program length must be a multiple of {} octets", ebpf::INSN_SIZE)] diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 15bf17240a..1866ac3830 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -50,7 +50,7 @@ impl DecodeError for BPFLoaderError { } /// Errors returned by functions the BPF Loader registers with the vM -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq)] pub enum BPFError { #[error("{0}")] VerifierError(#[from] VerifierError), @@ -253,9 +253,10 @@ pub fn process_instruction( mod tests { use super::*; use rand::Rng; + use solana_runtime::message_processor::ThisInvokeContext; use solana_sdk::{ account::Account, - entrypoint_native::{ComputeMeter, Logger, ProcessInstruction}, + entrypoint_native::{ComputeBudget, Logger, ProcessInstruction}, instruction::CompiledInstruction, message::Message, rent::Rent, @@ -332,6 +333,9 @@ mod tests { fn is_cross_program_supported(&self) -> bool { true } + fn get_compute_budget(&self) -> ComputeBudget { + ComputeBudget::default() + } fn get_compute_meter(&self) -> Rc> { Rc::new(RefCell::new(self.compute_meter.clone())) } @@ -562,6 +566,29 @@ mod tests { ) ); + // Case: limited budget + let program_id = Pubkey::default(); + let mut invoke_context = ThisInvokeContext::new( + &program_id, + Rent::default(), + vec![], + vec![], + None, + true, + ComputeBudget { + max_units: 1, + log_units: 100, + log_64_units: 100, + create_program_address_units: 1500, + invoke_units: 1000, + max_invoke_depth: 2, + }, + ); + assert_eq!( + Err(InstructionError::Custom(194969602)), + process_instruction(&bpf_loader::id(), &keyed_accounts, &[], &mut invoke_context) + ); + // Case: With duplicate accounts let duplicate_key = Pubkey::new_rand(); let parameter_account = Account::new_ref(1, 0, &program_id); diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index bd14ee5338..8e637a05fd 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -30,7 +30,7 @@ use std::{ use thiserror::Error as ThisError; /// Error definitions -#[derive(Debug, ThisError)] +#[derive(Debug, ThisError, PartialEq)] pub enum SyscallError { #[error("{0}: {1:?}")] InvalidString(Utf8Error, Vec), @@ -59,13 +59,6 @@ impl From for EbpfError { } } -/// Sysval compute costs -/// Note: `abort`, `sol_panic_`, and `sol_alloc_free_` do not currently incur a cost -const COMPUTE_COST_LOG: u64 = 100; -const COMPUTE_COST_LOG_64: u64 = 100; -const COMPUTE_COST_CREATE_PROGRAM_ADDRESS: u64 = 1000; -const COMPUTE_COST_INVOKE: u64 = 1000; - trait SyscallConsume { fn consume(&mut self, amount: u64) -> Result<(), EbpfError>; } @@ -97,6 +90,7 @@ pub fn register_syscalls<'a>( callers_keyed_accounts: &'a [KeyedAccount<'a>], invoke_context: &'a mut dyn InvokeContext, ) -> Result> { + let compute_budget = invoke_context.get_compute_budget(); // Syscall functions common across languages vm.register_syscall_ex("abort", syscall_abort)?; @@ -104,6 +98,7 @@ pub fn register_syscalls<'a>( vm.register_syscall_with_context_ex( "sol_log_", Box::new(SyscallLog { + cost: compute_budget.log_units, compute_meter: invoke_context.get_compute_meter(), logger: invoke_context.get_logger(), }), @@ -111,6 +106,7 @@ pub fn register_syscalls<'a>( vm.register_syscall_with_context_ex( "sol_log_64_", Box::new(SyscallLogU64 { + cost: compute_budget.log_64_units, compute_meter: invoke_context.get_compute_meter(), logger: invoke_context.get_logger(), }), @@ -119,6 +115,7 @@ pub fn register_syscalls<'a>( vm.register_syscall_with_context_ex( "sol_create_program_address", Box::new(SyscallCreateProgramAddress { + cost: compute_budget.create_program_address_units, compute_meter: invoke_context.get_compute_meter(), }), )?; @@ -282,6 +279,7 @@ pub fn syscall_sol_panic( /// Log a user's info message pub struct SyscallLog { + cost: u64, compute_meter: Rc>, logger: Rc>, } @@ -296,7 +294,7 @@ impl SyscallObject for SyscallLog { ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], ) -> Result> { - self.compute_meter.consume(COMPUTE_COST_LOG)?; + self.compute_meter.consume(self.cost)?; let mut logger = self .logger .try_borrow_mut() @@ -313,6 +311,7 @@ impl SyscallObject for SyscallLog { /// Log 5 64-bit values pub struct SyscallLogU64 { + cost: u64, compute_meter: Rc>, logger: Rc>, } @@ -327,7 +326,7 @@ impl SyscallObject for SyscallLogU64 { _ro_regions: &[MemoryRegion], _rw_regions: &[MemoryRegion], ) -> Result> { - self.compute_meter.consume(COMPUTE_COST_LOG_64)?; + self.compute_meter.consume(self.cost)?; let mut logger = self .logger .try_borrow_mut() @@ -386,6 +385,7 @@ impl SyscallObject for SyscallSolAllocFree { /// Create a program address pub struct SyscallCreateProgramAddress { + cost: u64, compute_meter: Rc>, } impl SyscallObject for SyscallCreateProgramAddress { @@ -399,8 +399,7 @@ impl SyscallObject for SyscallCreateProgramAddress { ro_regions: &[MemoryRegion], rw_regions: &[MemoryRegion], ) -> Result> { - self.compute_meter - .consume(COMPUTE_COST_CREATE_PROGRAM_ADDRESS)?; + self.compute_meter.consume(self.cost)?; let untranslated_seeds = translate_slice!(&[&u8], seeds_addr, seeds_len, ro_regions)?; let seeds = untranslated_seeds @@ -894,7 +893,7 @@ fn call<'a>( let mut invoke_context = syscall.get_context_mut()?; invoke_context .get_compute_meter() - .consume(COMPUTE_COST_INVOKE)?; + .consume(invoke_context.get_compute_budget().invoke_units)?; // Translate data passed from the VM @@ -1134,13 +1133,12 @@ mod tests { let addr = string.as_ptr() as *const _ as u64; let compute_meter: Rc> = - Rc::new(RefCell::new(MockComputeMeter { - remaining: std::u64::MAX, // TODO also test error - })); + Rc::new(RefCell::new(MockComputeMeter { remaining: 3 })); let log = Rc::new(RefCell::new(vec![])); let logger: Rc> = Rc::new(RefCell::new(MockLogger { log: log.clone() })); let mut syscall_sol_log = SyscallLog { + cost: 1, compute_meter, logger, }; @@ -1155,8 +1153,15 @@ mod tests { .call(100, string.len() as u64, 0, 0, 0, ro_regions, rw_regions) .unwrap(); - syscall_sol_log - .call( + assert_eq!( + Err(EbpfError::AccessViolation( + "programs/bpf_loader/src/syscalls.rs".to_string(), + 238, + 100, + 32, + " regions: \n0x64-0x73".to_string() + )), + syscall_sol_log.call( 100, string.len() as u64 * 2, // AccessViolation 0, @@ -1165,7 +1170,22 @@ mod tests { ro_regions, rw_regions, ) - .unwrap_err(); + ); + + assert_eq!( + Err(EbpfError::UserError(BPFError::SyscallError( + SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded) + ))), + syscall_sol_log.call( + 100, + string.len() as u64 * 2, // AccessViolation + 0, + 0, + 0, + ro_regions, + rw_regions, + ) + ); assert_eq!(log.borrow().len(), 1); assert_eq!(log.borrow()[0], "Program log: Gaggablaghblagh!"); @@ -1175,12 +1195,13 @@ mod tests { fn test_syscall_sol_log_u64() { let compute_meter: Rc> = Rc::new(RefCell::new(MockComputeMeter { - remaining: std::u64::MAX, // TODO also test error + remaining: std::u64::MAX, })); let log = Rc::new(RefCell::new(vec![])); let logger: Rc> = Rc::new(RefCell::new(MockLogger { log: log.clone() })); let mut syscall_sol_log_u64 = SyscallLogU64 { + cost: 0, compute_meter, logger, }; diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d1dff443d1..48d59b5d9b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -13,7 +13,7 @@ use crate::{ builtins::get_builtins, epoch_stakes::{EpochStakes, NodeVoteAccounts}, log_collector::LogCollector, - message_processor::{MessageProcessor, DEFAULT_COMPUTE_BUDGET, DEFAULT_MAX_INVOKE_DEPTH}, + message_processor::MessageProcessor, nonce_utils, rent_collector::RentCollector, stakes::Stakes, @@ -35,7 +35,7 @@ use solana_sdk::{ Epoch, Slot, SlotCount, SlotIndex, UnixTimestamp, DEFAULT_TICKS_PER_SECOND, MAX_PROCESSING_AGE, MAX_RECENT_BLOCKHASHES, SECONDS_PER_DAY, }, - entrypoint_native::{ProcessInstruction, ProcessInstructionWithContext}, + entrypoint_native::{ComputeBudget, ProcessInstruction, ProcessInstructionWithContext}, epoch_info::EpochInfo, epoch_schedule::EpochSchedule, fee_calculator::{FeeCalculator, FeeRateGovernor}, @@ -1245,13 +1245,8 @@ impl Bank { .set_cross_program_support(is_supported); } - pub fn set_max_invoke_depth(&mut self, max_invoke_depth: usize) { - self.message_processor - .set_max_invoke_depth(max_invoke_depth); - } - - pub fn set_compute_budget(&mut self, compute_units: u64) { - self.message_processor.set_compute_budget(compute_units); + pub fn set_compute_budget(&mut self, budget: ComputeBudget) { + self.message_processor.set_compute_budget(budget); } /// Return the last block hash registered. @@ -3209,8 +3204,7 @@ impl Bank { self.ensure_builtins(init_finish_or_warp); self.reinvoke_entered_epoch_callback(initiate_callback); self.recheck_cross_program_support(); - self.set_max_invoke_depth(DEFAULT_MAX_INVOKE_DEPTH); - self.set_compute_budget(DEFAULT_COMPUTE_BUDGET); + self.recheck_compute_budget(); } fn ensure_builtins(&mut self, init_or_warp: bool) { @@ -3239,6 +3233,27 @@ impl Bank { } } + fn recheck_compute_budget(self: &mut Bank) { + let compute_budget = if OperatingMode::Stable == self.operating_mode() { + if self.epoch() >= u64::MAX - 1 { + ComputeBudget::default() + } else { + // Original + ComputeBudget { + max_units: 100_000, + log_units: 0, + log_64_units: 0, + create_program_address_units: 0, + invoke_units: 0, + max_invoke_depth: 2, + } + } + } else { + ComputeBudget::default() + }; + self.set_compute_budget(compute_budget); + } + fn fix_recent_blockhashes_sysvar_delay(&self) -> bool { let activation_slot = match self.operating_mode() { OperatingMode::Development => 0, diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index fa488e3a6f..93fb96e6b8 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -7,8 +7,8 @@ use solana_sdk::{ account::{create_keyed_readonly_accounts, Account, KeyedAccount}, clock::Epoch, entrypoint_native::{ - ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext, InvokeContext, - Logger, ProcessInstruction, ProcessInstructionWithContext, + ComputeBudget, ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext, + InvokeContext, Logger, ProcessInstruction, ProcessInstructionWithContext, }, instruction::{CompiledInstruction, InstructionError}, message::Message, @@ -20,9 +20,6 @@ use solana_sdk::{ }; use std::{cell::RefCell, rc::Rc}; -pub const DEFAULT_MAX_INVOKE_DEPTH: usize = 2; -pub const DEFAULT_COMPUTE_BUDGET: u64 = 100_000; - // The relevant state of an account before an Instruction executes, used // to verify account integrity after the Instruction completes #[derive(Clone, Debug, Default)] @@ -183,7 +180,7 @@ pub struct ThisInvokeContext { programs: Vec<(Pubkey, ProcessInstruction)>, logger: Rc>, is_cross_program_supported: bool, - max_invoke_depth: usize, + compute_budget: ComputeBudget, compute_meter: Rc>, } impl ThisInvokeContext { @@ -194,10 +191,9 @@ impl ThisInvokeContext { programs: Vec<(Pubkey, ProcessInstruction)>, log_collector: Option>, is_cross_program_supported: bool, - max_invoke_depth: usize, - compute_budget: u64, + compute_budget: ComputeBudget, ) -> Self { - let mut program_ids = Vec::with_capacity(max_invoke_depth); + let mut program_ids = Vec::with_capacity(compute_budget.max_invoke_depth); program_ids.push(*program_id); Self { program_ids, @@ -206,16 +202,16 @@ impl ThisInvokeContext { programs, logger: Rc::new(RefCell::new(ThisLogger { log_collector })), is_cross_program_supported, - max_invoke_depth, + compute_budget, compute_meter: Rc::new(RefCell::new(ThisComputeMeter { - remaining: compute_budget, + remaining: compute_budget.max_units, })), } } } impl InvokeContext for ThisInvokeContext { fn push(&mut self, key: &Pubkey) -> Result<(), InstructionError> { - if self.program_ids.len() >= self.max_invoke_depth { + if self.program_ids.len() >= self.compute_budget.max_invoke_depth { return Err(InstructionError::CallDepth); } if self.program_ids.contains(key) && self.program_ids.last() != Some(key) { @@ -260,6 +256,9 @@ impl InvokeContext for ThisInvokeContext { fn is_cross_program_supported(&self) -> bool { self.is_cross_program_supported } + fn get_compute_budget(&self) -> ComputeBudget { + self.compute_budget + } fn get_compute_meter(&self) -> Rc> { self.compute_meter.clone() } @@ -290,9 +289,7 @@ pub struct MessageProcessor { #[serde(skip)] is_cross_program_supported: bool, #[serde(skip)] - max_invoke_depth: usize, - #[serde(skip)] - compute_budget: u64, + compute_budget: ComputeBudget, } impl std::fmt::Debug for MessageProcessor { @@ -339,11 +336,7 @@ impl Default for MessageProcessor { loaders: vec![], native_loader: NativeLoader::default(), is_cross_program_supported: true, - // Maximum cross-program invocation depth allowed including the orignal caller - max_invoke_depth: DEFAULT_MAX_INVOKE_DEPTH, - // Number of compute units that an instruction is allowed. Compute units - // are consumed by program execution, resources they use, etc... - compute_budget: DEFAULT_COMPUTE_BUDGET, + compute_budget: ComputeBudget::default(), } } } @@ -391,11 +384,7 @@ impl MessageProcessor { self.is_cross_program_supported = is_supported; } - pub fn set_max_invoke_depth(&mut self, max_invoke_depth: usize) { - self.max_invoke_depth = max_invoke_depth; - } - - pub fn set_compute_budget(&mut self, compute_budget: u64) { + pub fn set_compute_budget(&mut self, compute_budget: ComputeBudget) { self.compute_budget = compute_budget; } @@ -651,7 +640,6 @@ impl MessageProcessor { self.programs.clone(), // get rid of clone log_collector, self.is_cross_program_supported, - self.max_invoke_depth, self.compute_budget, ); let keyed_accounts = @@ -742,8 +730,7 @@ mod tests { vec![], None, true, - DEFAULT_MAX_INVOKE_DEPTH, - DEFAULT_COMPUTE_BUDGET, + ComputeBudget::default(), ); // Check call depth increases and has a limit @@ -1514,8 +1501,7 @@ mod tests { vec![], None, true, - DEFAULT_MAX_INVOKE_DEPTH, - DEFAULT_COMPUTE_BUDGET, + ComputeBudget::default(), ); let metas = vec![ AccountMeta::new(owned_key, false), diff --git a/sdk/src/entrypoint_native.rs b/sdk/src/entrypoint_native.rs index 9e7a057d78..ada453f49f 100644 --- a/sdk/src/entrypoint_native.rs +++ b/sdk/src/entrypoint_native.rs @@ -213,10 +213,43 @@ pub trait InvokeContext { fn get_logger(&self) -> Rc>; /// Are cross program invocations supported fn is_cross_program_supported(&self) -> bool; + /// Get this invocation's compute budget + fn get_compute_budget(&self) -> ComputeBudget; /// Get this invocation's compute meter fn get_compute_meter(&self) -> Rc>; } +#[derive(Clone, Copy, Debug)] +pub struct ComputeBudget { + /// Number of compute units that an instruction is allowed. Compute units + /// are consumed by program execution, resources they use, etc... + pub max_units: u64, + /// Number of compute units consumed by a log call + pub log_units: u64, + /// Number of compute units consumed by a log_u64 call + pub log_64_units: u64, + /// Number of compute units consumed by a create_program_address call + pub create_program_address_units: u64, + /// Number of compute units consumed by an invoke call (not including the cost incured by + /// the called program) + pub invoke_units: u64, + /// Maximum cross-program invocation depth allowed including the orignal caller + pub max_invoke_depth: usize, +} +impl Default for ComputeBudget { + fn default() -> Self { + // Tuned for ~1ms + ComputeBudget { + max_units: 200_000, + log_units: 100, + log_64_units: 100, + create_program_address_units: 1500, + invoke_units: 1000, + max_invoke_depth: 2, + } + } +} + /// Compute meter pub trait ComputeMeter { /// Consume compute units