diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index bcff3fa614ea05..6477df38bc0070 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -108,6 +108,67 @@ impl ComputeMeter { } } +// bprumo TODO: doc where 128 GB came from. Move where the const is, also +// 128 GB +pub const DEFAULT_MAX_ACCOUNTS_DATA_LEN: usize = 128_000_000_000; + +/// bprumo TODO: doc, and/or move +#[derive(Debug, Default, Clone, Copy, Eq, PartialEq)] +pub struct AccountsDataBudget { + /// The global maximum size for accounts data (i.e. DEFAULT_MAX_ACCOUNTS_DATA_LEN) + maximum: usize, + /// The current accounts data size (for a Bank) + current: usize, +} +impl AccountsDataBudget { + pub fn new(maximum: usize, current: usize) -> Self { + Self { maximum, current } + } + pub fn maximum(&self) -> usize { + self.maximum + } + pub fn current(&self) -> usize { + self.current + } + pub fn remaining(&self) -> usize { + self.maximum().saturating_sub(self.current()) + } +} + +/// bprumo TODO: doc, and/or move +#[derive(Debug, Default, Clone, Copy, Eq, PartialEq)] +pub struct AccountsDataMeter { + /// The amount of available accounts data space (i.e. AccountsDataBudget::remaining()) + capacity: usize, + /// The amount available of accounts data space consumed. This value is used to update the + /// Bank after transactions are successfully processed. + consumed: usize, +} +impl AccountsDataMeter { + pub fn new_ref(capacity: usize) -> Rc> { + Rc::new(RefCell::new(Self { + capacity, + consumed: 0, + })) + } + pub fn capacity(&self) -> usize { + self.capacity + } + pub fn consumed(&self) -> usize { + self.consumed + } + pub fn remaining(&self) -> usize { + self.capacity().saturating_sub(self.consumed()) + } + pub fn consume(&mut self, amount: usize) -> Result<(), InstructionError> { + if amount > self.remaining() { + return Err(InstructionError::AccountsDataBudgetExceeded); + } + self.consumed = self.consumed.saturating_add(amount); + Ok(()) + } +} + pub struct StackFrame<'a> { pub number_of_program_accounts: usize, pub keyed_accounts: Vec>, @@ -134,6 +195,7 @@ impl<'a> StackFrame<'a> { } } +// bprumo NOTE: here's the invoke context pub struct InvokeContext<'a> { invoke_stack: Vec>, rent: Rent, @@ -145,6 +207,7 @@ pub struct InvokeContext<'a> { compute_budget: ComputeBudget, current_compute_budget: ComputeBudget, compute_meter: Rc>, + accounts_data_meter: Rc>, executors: Rc>, pub instruction_recorder: Option<&'a InstructionRecorder>, pub feature_set: Arc, @@ -163,6 +226,7 @@ impl<'a> InvokeContext<'a> { sysvars: &'a [(Pubkey, Vec)], log_collector: Option>>, compute_budget: ComputeBudget, + accounts_data_budget: AccountsDataBudget, executors: Rc>, feature_set: Arc, blockhash: Hash, @@ -179,6 +243,7 @@ impl<'a> InvokeContext<'a> { current_compute_budget: compute_budget, compute_budget, compute_meter: ComputeMeter::new_ref(compute_budget.max_units), + accounts_data_meter: AccountsDataMeter::new_ref(accounts_data_budget.remaining()), executors, instruction_recorder: None, feature_set, @@ -200,6 +265,7 @@ impl<'a> InvokeContext<'a> { &[], Some(LogCollector::new_ref()), ComputeBudget::default(), + AccountsDataBudget::new(DEFAULT_MAX_ACCOUNTS_DATA_LEN, 0), Rc::new(RefCell::new(Executors::default())), Arc::new(FeatureSet::all_enabled()), Hash::default(), @@ -508,6 +574,7 @@ impl<'a> InvokeContext<'a> { // Verify the called program has not misbehaved let do_support_realloc = self.feature_set.is_active(&do_support_realloc::id()); for (account, prev_size) in prev_account_sizes.iter() { + // bprumo NOTE: looks like this checks for size changes... if !do_support_realloc && *prev_size != account.borrow().data().len() && *prev_size != 0 { // Only support for `CreateAccount` at this time. @@ -636,6 +703,7 @@ impl<'a> InvokeContext<'a> { Ok((message, caller_write_privileges, program_indices)) } + // bprumo NOTE: here's where the CPI instructions are processed /// Process a cross-program instruction pub fn process_instruction( &mut self, @@ -783,6 +851,11 @@ impl<'a> InvokeContext<'a> { self.compute_meter.clone() } + /// bprumo TODO: doc + pub fn accounts_data_meter(&self) -> Rc> { + Rc::clone(&self.accounts_data_meter) + } + /// Loaders may need to do work in order to execute a program. Cache /// the work that can be re-used across executions pub fn add_executor(&self, pubkey: &Pubkey, executor: Arc) { diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index df9078bd64a12e..652c4a7a98c19d 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -1518,6 +1518,14 @@ struct IndexAccountMapEntry<'a> { type GenerateIndexAccountsMap<'a> = HashMap>; +#[derive(Debug, Default, Clone, Copy)] +struct GenerateIndexForSlotResult { + insert_time_us: u64, + num_accounts: u64, + num_accounts_rent_exempt: u64, + accounts_data_len: usize, +} + impl AccountsDb { pub fn default_for_tests() -> Self { Self::default_with_accounts_index(AccountInfoAccountsIndex::default_for_tests(), None, None) @@ -6657,21 +6665,21 @@ impl AccountsDb { accounts_map } - /// return time_us, # accts rent exempt, total # accts fn generate_index_for_slot<'a>( &self, accounts_map: GenerateIndexAccountsMap<'a>, slot: &Slot, rent_collector: &RentCollector, - ) -> (u64, u64, u64) { + ) -> GenerateIndexForSlotResult { if accounts_map.is_empty() { - return (0, 0, 0); + return GenerateIndexForSlotResult::default(); } let secondary = !self.account_indexes.is_empty(); - let mut rent_exempt = 0; - let len = accounts_map.len(); + let mut num_accounts_rent_exempt = 0; + let mut accounts_data_len = 0; + let num_accounts = accounts_map.len(); let items = accounts_map.into_iter().map( |( pubkey, @@ -6689,12 +6697,15 @@ impl AccountsDb { &self.account_indexes, ); } + if !stored_account.is_zero_lamport() { + accounts_data_len += stored_account.data().len(); + } if !rent_collector.should_collect_rent(&pubkey, &stored_account, false) || { let (_rent_due, exempt) = rent_collector.get_rent_due(&stored_account); exempt } { - rent_exempt += 1; + num_accounts_rent_exempt += 1; } ( @@ -6709,9 +6720,9 @@ impl AccountsDb { }, ); - let (dirty_pubkeys, insert_us) = self + let (dirty_pubkeys, insert_time_us) = self .accounts_index - .insert_new_if_missing_into_primary_index(*slot, len, items); + .insert_new_if_missing_into_primary_index(*slot, num_accounts, items); // dirty_pubkeys will contain a pubkey if an item has multiple rooted entries for // a given pubkey. If there is just a single item, there is no cleaning to @@ -6719,7 +6730,12 @@ impl AccountsDb { if !dirty_pubkeys.is_empty() { self.uncleaned_pubkeys.insert(*slot, dirty_pubkeys); } - (insert_us, rent_exempt, len as u64) + GenerateIndexForSlotResult { + insert_time_us, + num_accounts: num_accounts as u64, + num_accounts_rent_exempt, + accounts_data_len, + } } fn filler_unique_id_bytes() -> usize { @@ -6855,7 +6871,7 @@ impl AccountsDb { limit_load_slot_count_from_snapshot: Option, verify: bool, genesis_config: &GenesisConfig, - ) { + ) -> usize { let mut slots = self.storage.all_slots(); #[allow(clippy::stable_sort_primitive)] slots.sort(); @@ -6870,6 +6886,7 @@ impl AccountsDb { genesis_config.slots_per_year(), &genesis_config.rent, ); + let accounts_data_len = AtomicUsize::new(0); // pass == 0 always runs and generates the index // pass == 1 only runs if verify == true. @@ -6915,10 +6932,16 @@ impl AccountsDb { let insert_us = if pass == 0 { // generate index - let (insert_us, rent_exempt_this_slot, total_this_slot) = - self.generate_index_for_slot(accounts_map, slot, &rent_collector); + let GenerateIndexForSlotResult { + insert_time_us: insert_us, + num_accounts: total_this_slot, + num_accounts_rent_exempt: rent_exempt_this_slot, + accounts_data_len: accounts_data_len_this_slot, + } = self.generate_index_for_slot(accounts_map, slot, &rent_collector); rent_exempt.fetch_add(rent_exempt_this_slot, Ordering::Relaxed); total_duplicates.fetch_add(total_this_slot, Ordering::Relaxed); + accounts_data_len + .fetch_add(accounts_data_len_this_slot, Ordering::Relaxed); insert_us } else { // verify index matches expected and measure the time to get all items @@ -6972,6 +6995,69 @@ impl AccountsDb { }) .sum(); + if pass == 0 { + // subtract data.len() from accounts_data_len for all old accounts which are in the index twice + let mut timer = Measure::start("handle accounts data len duplicates"); + let mut unique_pubkeys = HashSet::::default(); + self.uncleaned_pubkeys.iter().for_each(|entry| { + entry.value().iter().for_each(|pubkey| { + unique_pubkeys.insert(*pubkey); + }) + }); + unique_pubkeys + .into_iter() + .collect::>() + .par_chunks(4096) + .for_each(|pubkeys| { + // subract out older data.len() for each pubkey + let mut accounts_data_len_from_duplicates = 0; + pubkeys.into_iter().for_each(|pubkey| { + if let Some(entry) = self.accounts_index.get_account_read_entry(pubkey) + { + let list = entry.slot_list(); + if list.len() < 2 { + return; + } + let mut list = list.clone(); + list.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + assert!(list[0].0 < list[1].0); // greatest to least + list.into_iter() + .rev() + .skip(1) + .for_each(|(slot, account_info)| { + let maybe_storage_entry = self + .storage + .get_account_storage_entry(slot, account_info.store_id); + let mut accessor = LoadedAccountAccessor::Stored( + maybe_storage_entry + .map(|entry| (entry, account_info.offset)), + ); + let loaded_account = + accessor.check_and_get_loaded_account(); + let account = loaded_account.take_account(); + if !account.is_zero_lamport() { + accounts_data_len_from_duplicates += + account.data().len(); + } + }); + } + }); + accounts_data_len + .fetch_sub(accounts_data_len_from_duplicates, Ordering::Relaxed); + }); + timer.stop(); + info!( + "accounts data len: {}, {}", + accounts_data_len.load(Ordering::Relaxed), + timer + ); + error!( + "bprumo DEBUG: generate_index(), accounts data len: {}, {}", + accounts_data_len.load(Ordering::Relaxed), + timer + ); + } + let storage_info_timings = storage_info_timings.into_inner().unwrap(); let mut index_flush_us = 0; @@ -7009,6 +7095,7 @@ impl AccountsDb { } timings.report(); } + accounts_data_len.load(Ordering::Relaxed) } fn update_storage_info( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 22ec96492dec28..ec004040baa0e6 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -74,7 +74,10 @@ use { solana_metrics::{inc_new_counter_debug, inc_new_counter_info}, solana_program_runtime::{ instruction_recorder::InstructionRecorder, - invoke_context::{BuiltinProgram, Executor, Executors, ProcessInstructionWithContext}, + invoke_context::{ + AccountsDataBudget, BuiltinProgram, Executor, Executors, ProcessInstructionWithContext, + DEFAULT_MAX_ACCOUNTS_DATA_LEN, + }, log_collector::LogCollector, timings::ExecuteDetailsTimings, }, @@ -117,8 +120,7 @@ use { signature::{Keypair, Signature}, slot_hashes::SlotHashes, slot_history::SlotHistory, - system_transaction, - sysvar::{self}, + system_transaction, sysvar, timing::years_as_slots, transaction::{ Result, SanitizedTransaction, Transaction, TransactionError, @@ -143,7 +145,10 @@ use { ptr, rc::Rc, sync::{ - atomic::{AtomicBool, AtomicU64, Ordering::Relaxed}, + atomic::{ + AtomicBool, AtomicU64, AtomicUsize, + Ordering::{AcqRel, Acquire, Relaxed, Release}, + }, Arc, LockResult, RwLock, RwLockReadGuard, RwLockWriteGuard, }, time::{Duration, Instant}, @@ -733,6 +738,7 @@ pub(crate) struct BankFieldsToDeserialize { pub(crate) stakes: Stakes, pub(crate) epoch_stakes: HashMap, pub(crate) is_delta: bool, + // bprumo TODO: pub(crate) accounts_data_len: usize, } // Bank's common fields shared by all supported snapshot versions for serialization. @@ -772,6 +778,7 @@ pub(crate) struct BankFieldsToSerialize<'a> { pub(crate) stakes: &'a RwLock, pub(crate) epoch_stakes: &'a HashMap, pub(crate) is_delta: bool, + // bprumo TODO: pub(crate) accounts_data_len: usize, } // Can't derive PartialEq because RwLock doesn't implement PartialEq @@ -1040,6 +1047,9 @@ pub struct Bank { pub cost_tracker: RwLock, sysvar_cache: RwLock)>>, + + /// bprumo TODO: doc + accounts_data_len: AtomicUsize, } impl Default for BlockhashQueue { @@ -1119,7 +1129,7 @@ impl Bank { } fn default_with_accounts(accounts: Accounts) -> Self { - Self { + let bank = Self { rc: BankRc::new(accounts, Slot::default()), src: StatusCacheRc::default(), blockhash_queue: RwLock::::default(), @@ -1175,7 +1185,16 @@ impl Bank { vote_only_bank: false, cost_tracker: RwLock::::default(), sysvar_cache: RwLock::new(Vec::new()), - } + accounts_data_len: AtomicUsize::default(), + }; + + // bprumo TODO: how often is default_with_accounts() called? If I call scan() here, is that + // going to be way too slow? + let total_account_stats = bank.get_total_accounts_stats().unwrap(); + bank.accounts_data_len + .store(total_account_stats.data_len, Release); + + bank } pub fn new_with_paths_for_tests( @@ -1421,8 +1440,17 @@ impl Bank { freeze_started: AtomicBool::new(false), cost_tracker: RwLock::new(CostTracker::default()), sysvar_cache: RwLock::new(Vec::new()), + accounts_data_len: AtomicUsize::new(parent.accounts_data_len.load(Acquire)), }; + // bprumo TODO: move this somewhere else, like bank deserialization (from snapshot) + if new.accounts_data_len.load(Acquire) == 0 { + error!("bprumo DEBUG: Bank::new_from_parent(), parent accounts_data_len == 0!"); + let total_account_stats = new.get_total_accounts_stats().unwrap(); + new.accounts_data_len + .store(total_account_stats.data_len, Release); + } + let mut ancestors = Vec::with_capacity(1 + new.parents().len()); ancestors.push(new.slot()); new.parents().iter().for_each(|p| { @@ -1536,6 +1564,7 @@ impl Bank { debug_keys: Option>>, additional_builtins: Option<&Builtins>, debug_do_not_add_builtins: bool, + accounts_data_len: usize, ) -> Self { fn new() -> T { T::default() @@ -1598,6 +1627,7 @@ impl Bank { vote_only_bank: false, cost_tracker: RwLock::new(CostTracker::default()), sysvar_cache: RwLock::new(Vec::new()), + accounts_data_len: AtomicUsize::new(accounts_data_len), }; bank.finish_init( genesis_config, @@ -1680,6 +1710,7 @@ impl Bank { stakes: &self.stakes, epoch_stakes: &self.epoch_stakes, is_delta: self.is_delta.load(Relaxed), + // bprumo TODO: accounts_data_len: self.accounts_data_len.load(Acquire), } } @@ -3435,6 +3466,7 @@ impl Bank { cache.remove(pubkey); } + // bprumo NOTE: here's where the bank executes the txns #[allow(clippy::type_complexity)] pub fn load_and_execute_transactions( &self, @@ -3514,6 +3546,7 @@ impl Bank { let feature_set = self.feature_set.clone(); signature_count += u64::from(tx.message().header().num_required_signatures); + // bprumo NOTE: Here's where the ComputeBudget is created let mut compute_budget = self.compute_budget.unwrap_or_else(ComputeBudget::new); let mut process_result = if feature_set.is_active(&tx_wide_compute_cap::id()) { @@ -3551,6 +3584,7 @@ impl Bank { self.last_blockhash_and_lamports_per_signature(); if let Some(legacy_message) = tx.message().legacy_message() { + // bprumo NOTE: Here's where the message/transaction starts getting processed process_result = MessageProcessor::process_message( &self.builtin_programs.vec, legacy_message, @@ -3562,11 +3596,22 @@ impl Bank { instruction_recorders.as_deref(), feature_set, compute_budget, + AccountsDataBudget::new( + DEFAULT_MAX_ACCOUNTS_DATA_LEN, + self.accounts_data_len.load(Acquire), + ), &mut timings.details, &*self.sysvar_cache.read().unwrap(), blockhash, lamports_per_signature, - ); + ) + .and_then(|process_message_result| { + self.accounts_data_len.fetch_add( + process_message_result.accounts_data_meter.consumed(), + AcqRel, + ); + Ok(()) + }); } else { // TODO: support versioned messages process_result = Err(TransactionError::UnsupportedVersion); diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index debe01f3e90605..3dc9c70bd1b7e0 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -3,7 +3,9 @@ use { solana_measure::measure::Measure, solana_program_runtime::{ instruction_recorder::InstructionRecorder, - invoke_context::{BuiltinProgram, Executors, InvokeContext}, + invoke_context::{ + AccountsDataBudget, AccountsDataMeter, BuiltinProgram, Executors, InvokeContext, + }, log_collector::LogCollector, timings::ExecuteDetailsTimings, }, @@ -34,6 +36,12 @@ impl ::solana_frozen_abi::abi_example::AbiExample for MessageProcessor { } } +/// bprumo TODO: doc +#[derive(Debug, Eq, PartialEq)] +pub struct ProcessMessageResult { + pub accounts_data_meter: AccountsDataMeter, +} + impl MessageProcessor { /// Process a message. /// This method calls each instruction in the message over the set of loaded accounts. @@ -52,11 +60,13 @@ impl MessageProcessor { instruction_recorders: Option<&[InstructionRecorder]>, feature_set: Arc, compute_budget: ComputeBudget, + accounts_data_budget: AccountsDataBudget, timings: &mut ExecuteDetailsTimings, sysvars: &[(Pubkey, Vec)], blockhash: Hash, lamports_per_signature: u64, - ) -> Result<(), TransactionError> { + ) -> Result { + // bprumo NOTE: Here's where InvokeContext is created let mut invoke_context = InvokeContext::new( rent, accounts, @@ -64,6 +74,7 @@ impl MessageProcessor { sysvars, log_collector, compute_budget, + accounts_data_budget, executors, feature_set, blockhash, @@ -106,6 +117,7 @@ impl MessageProcessor { } let pre_remaining_units = invoke_context.get_compute_meter().borrow().get_remaining(); let mut time = Measure::start("execute_instruction"); + // bprumo NOTE: here's where the instruction is processed invoke_context .process_instruction(message, instruction, program_indices, &[], &[]) .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; @@ -118,7 +130,9 @@ impl MessageProcessor { ); timings.accumulate(&invoke_context.timings); } - Ok(()) + Ok(ProcessMessageResult { + accounts_data_meter: invoke_context.accounts_data_meter().take(), + }) } } @@ -239,12 +253,13 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), 0, ); - assert_eq!(result, Ok(())); + assert!(result.is_ok()); assert_eq!(accounts[0].1.borrow().lamports(), 100); assert_eq!(accounts[1].1.borrow().lamports(), 0); @@ -268,6 +283,7 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), @@ -301,6 +317,7 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), @@ -445,6 +462,7 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), @@ -478,12 +496,13 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), 0, ); - assert_eq!(result, Ok(())); + assert!(result.is_ok()); // Do work on the same account but at different location in keyed_accounts[] let message = Message::new( @@ -508,12 +527,13 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), 0, ); - assert_eq!(result, Ok(())); + assert!(result.is_ok()); assert_eq!(accounts[0].1.borrow().lamports(), 80); assert_eq!(accounts[1].1.borrow().lamports(), 20); assert_eq!(accounts[0].1.borrow().data(), &vec![42]); @@ -565,6 +585,7 @@ mod tests { None, Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), + AccountsDataBudget::default(), &mut ExecuteDetailsTimings::default(), &[], Hash::default(), diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index 600d8b823a2d79..b0c67bb47e4188 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -334,7 +334,7 @@ fn reconstruct_bank_from_fields( where E: SerializableStorage + std::marker::Sync, { - let accounts_db = reconstruct_accountsdb_from_fields( + let (accounts_db, accounts_data_len) = reconstruct_accountsdb_from_fields( snapshot_accounts_db_fields, account_paths, unpacked_append_vec_map, @@ -359,6 +359,7 @@ where debug_keys, additional_builtins, debug_do_not_add_builtins, + accounts_data_len, ); info!("rent_collector: {:?}", bank.rent_collector()); @@ -399,7 +400,7 @@ fn reconstruct_accountsdb_from_fields( verify_index: bool, accounts_db_config: Option, accounts_update_notifier: Option, -) -> Result +) -> Result<(AccountsDb, usize), Error> where E: SerializableStorage + std::marker::Sync, { @@ -536,7 +537,7 @@ where }) .unwrap(); - accounts_db.generate_index( + let accounts_data_len = accounts_db.generate_index( limit_load_slot_count_from_snapshot, verify_index, genesis_config, @@ -557,5 +558,5 @@ where ("accountsdb-notify-at-start-us", measure_notify.as_us(), i64), ); - Ok(Arc::try_unwrap(accounts_db).unwrap()) + Ok((Arc::try_unwrap(accounts_db).unwrap(), accounts_data_len)) } diff --git a/runtime/src/serde_snapshot/future.rs b/runtime/src/serde_snapshot/future.rs index 8fd4a9a455ff04..b7c069d93058ce 100644 --- a/runtime/src/serde_snapshot/future.rs +++ b/runtime/src/serde_snapshot/future.rs @@ -81,6 +81,7 @@ pub(crate) struct DeserializableVersionedBank { pub(crate) unused_accounts: UnusedAccounts, pub(crate) epoch_stakes: HashMap, pub(crate) is_delta: bool, + // bprumo TODO: pub(crate) accounts_data_len: usize, } impl From for BankFieldsToDeserialize { @@ -117,6 +118,7 @@ impl From for BankFieldsToDeserialize { stakes: dvb.stakes, epoch_stakes: dvb.epoch_stakes, is_delta: dvb.is_delta, + // bprumo TODO: accounts_data_len: dvb.accounts_data_len, } } } @@ -157,6 +159,7 @@ pub(crate) struct SerializableVersionedBank<'a> { pub(crate) unused_accounts: UnusedAccounts, pub(crate) epoch_stakes: &'a HashMap, pub(crate) is_delta: bool, + // bprumo TODO: pub(crate) accounts_data_len: usize, } impl<'a> From> for SerializableVersionedBank<'a> { @@ -197,6 +200,7 @@ impl<'a> From> for SerializableVersionedB unused_accounts: new(), epoch_stakes: rhs.epoch_stakes, is_delta: rhs.is_delta, + // bprumo TODO: accounts_data_len: rhs.accounts_data_len, } } } diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index 5161af7842c180..50e5d9539f4af3 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -102,7 +102,15 @@ fn allocate( return Err(SystemError::InvalidAccountDataLength.into()); } - account.set_data(vec![0; space as usize]); + // bprumo NOTE: Maybe here is the right spot to check if the new account data length exceeds + // the max total account data size? + let space = space as usize; + invoke_context + .accounts_data_meter() + .borrow_mut() + .consume(space)?; + + account.set_data(vec![0; space]); Ok(()) } @@ -272,6 +280,7 @@ fn transfer_with_seed( transfer_verified(from, to, lamports, invoke_context) } +// bprumo NOTE: here's the process instruction, where I can add stuff to the match {} pub fn process_instruction( first_instruction_account: usize, instruction_data: &[u8], @@ -796,6 +805,42 @@ mod tests { ); } + #[test] + fn test_request_more_than_allowed_accounts_data_length() { + solana_logger::setup(); + let invoke_context = InvokeContext::new_mock(&[], &[]); + + let result = loop { + let from_account = AccountSharedData::new_ref(100, 0, &system_program::id()); + let from = Pubkey::new_unique(); + let to_account = AccountSharedData::new_ref(0, 0, &system_program::id()); + let to = Pubkey::new_unique(); + + let signers = &[from, to].iter().cloned().collect::>(); + let address = &to.into(); + + let result = create_account( + &KeyedAccount::new(&from, true, &from_account), + &KeyedAccount::new(&to, false, &to_account), + address, + 10, + MAX_PERMITTED_DATA_LENGTH, + &system_program::id(), + signers, + &invoke_context, + ); + + if result.is_err() { + break result; + } + }; + + assert_eq!( + result.err().unwrap(), + InstructionError::AccountsDataBudgetExceeded + ); + } + #[test] fn test_create_already_in_use() { let invoke_context = InvokeContext::new_mock(&[], &[]); diff --git a/sdk/program/src/instruction.rs b/sdk/program/src/instruction.rs index 51324ab60953b3..30bdbd3eb8be84 100644 --- a/sdk/program/src/instruction.rs +++ b/sdk/program/src/instruction.rs @@ -236,6 +236,10 @@ pub enum InstructionError { /// Illegal account owner #[error("Provided owner is not allowed")] IllegalOwner, + + /// bprumo TODO: doc here, and add equivalent ProgramError + #[error("Accounts data budget exceeded")] + AccountsDataBudgetExceeded, // Note: For any new error added here an equivalent ProgramError and its // conversions must also be added } diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index 36c17832ee9b85..140d719b2596bd 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -104,6 +104,7 @@ enum InstructionErrorType { ARITHMETIC_OVERFLOW = 47; UNSUPPORTED_SYSVAR = 48; ILLEGAL_OWNER = 49; + ACCOUNTS_DATA_BUDGET_EXCEEDED = 50; } message UnixTimestamp { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index e8511867c6b3a8..72bdf72938761e 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -791,6 +791,9 @@ impl From for tx_by_addr::TransactionError { InstructionError::IllegalOwner => { tx_by_addr::InstructionErrorType::IllegalOwner } + InstructionError::AccountsDataBudgetExceeded => { + tx_by_addr::InstructionErrorType::IllegalOwner + } } as i32, custom: match instruction_error { InstructionError::Custom(custom) => {