diff --git a/Cargo.toml b/Cargo.toml index e4e62d95..b4211052 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["solana/account", "solana/transaction-view"] +members = ["solana/account", "solana/transaction-context", "solana/transaction-view"] resolver = "3" [workspace.package] @@ -15,6 +15,7 @@ version = "0.1.0" magic-root-interface = { path = "programs/magic-root-interface" } magic-root-program = { path = "programs/magic-root-program" } solana-account = { path = "solana/account" } +solana-transaction-context = { path = "solana/transaction-context" } ahash = "0.8.12" arc-swap = "1.9.1" @@ -36,8 +37,10 @@ tracing-subscriber = { version = "0.3.23", features = ["env-filter", "fmt"] } wincode = "0.5.1" zstd = { version = "0.13.3", default-features = false } -agave-feature-set = { version = "3.1.14", features = ["agave-unstable-api"] } -agave-transaction-view = { version = "4.1.1", features = ["agave-unstable-api"] } +agave-feature-set = "4.1.1" +agave-precompiles = "4.1.1" +agave-syscalls = { package = "solana-syscalls", version = "=4.1.1", default-features = false } +agave-transaction-view = "4.1.1" solana-account-info = "3.1.1" solana-clock = "3.1.0" solana-compute-budget-instruction = "=4.1.1" @@ -45,12 +48,16 @@ solana-cpi = "3.1.0" solana-hash = "4.3.0" solana-instruction = "=3.4.0" solana-instruction-error = "=2.4.0" +solana-instructions-sysvar = "4.0.0" solana-keypair = "3.1.2" solana-message = "4.1.1" solana-packet = "=4.2.0" +solana-program-entrypoint = "3.1.1" solana-program-runtime = { version = "=4.1.1", features = ["agave-unstable-api"] } solana-program-error = "3.0.1" solana-pubkey = "=4.2.0" +solana-rent = "4.0.0-rc.1" +solana-sbpf = "0.13.1" solana-sdk-ids = "3.1.0" solana-short-vec = "=3.2.2" solana-signature = "=3.4.1" @@ -59,7 +66,6 @@ solana-svm-transaction = "4.1.1" solana-system-interface = { version = "=3.2.0", features = ["alloc", "bincode", "serde", "wincode"] } solana-sysvar = "4.0.0" solana-transaction = "4.1.1" -solana-transaction-context = "4.1.1" solana-transaction-error = "=3.3.1" [patch.crates-io] diff --git a/solana/transaction-context/Cargo.toml b/solana/transaction-context/Cargo.toml index f9cc89ea..0209282e 100644 --- a/solana/transaction-context/Cargo.toml +++ b/solana/transaction-context/Cargo.toml @@ -1,32 +1,34 @@ [package] name = "solana-transaction-context" + +authors = { workspace = true } description = "Solana data shared between program runtime and built-in programs as well as SBF programs." documentation = "https://docs.rs/solana-transaction-context" -version = { workspace = true } -authors = { workspace = true } -repository = { workspace = true } +edition = { workspace = true } homepage = { workspace = true } license = { workspace = true } -edition = "2024" +repository = { workspace = true } +version = { workspace = true } [package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] all-features = true rustdoc-args = ["--cfg=docsrs"] +targets = ["x86_64-unknown-linux-gnu"] [features] +# No-op stub retained only so external (patched-in) crates that reference +# `solana-transaction-context/agave-unstable-api` still resolve; the lib is no +# longer gated on it. agave-unstable-api = [] bincode = ["dep:bincode", "serde", "solana-account/bincode"] -dev-context-only-utils = ["bincode", "solana-account/dev-context-only-utils", "dep:qualifier_attr"] +dev-context-only-utils = ["bincode"] serde = ["serde/derive", "solana-pubkey/serde"] -wincode = ["dep:wincode", "solana-pubkey/wincode"] [dependencies] solana-account = { workspace = true } solana-instruction = { workspace = true, features = ["std"] } solana-instructions-sysvar = { workspace = true } solana-pubkey = { workspace = true } -wincode = { workspace = true, optional = true } [target.'cfg(not(any(target_arch = "sbf", target_arch = "bpf")))'.dependencies] bincode = { workspace = true, optional = true } @@ -39,11 +41,8 @@ solana-sdk-ids = { workspace = true } solana-account-info = { workspace = true } solana-program-entrypoint = { workspace = true } solana-system-interface = { workspace = true } -solana-transaction-context = { path = ".", features = [ - "agave-unstable-api", - "dev-context-only-utils", -] } -static_assertions = { workspace = true } +solana-transaction-context = { path = ".", features = ["dev-context-only-utils"] } +static_assertions = "1.1.0" -[lints] -workspace = true +[lints.rust] +unexpected_cfgs = "allow" diff --git a/solana/transaction-context/README.md b/solana/transaction-context/README.md new file mode 100644 index 00000000..e2936917 --- /dev/null +++ b/solana/transaction-context/README.md @@ -0,0 +1,15 @@ +# `solana-transaction-context` + +This Agave fork defines the account and instruction state used by one executing +transaction. Workspace `[patch.crates-io]` entries force the dependency graph to +use this copy. + +`TransactionAccounts` stores account cells behind runtime borrow counters. +`AccountRef` and `AccountRefMut` enforce those counters while VM access handlers +can resize and remap an account's directly mapped data. The context also tracks +touched accounts, resize and lamport deltas, return data, instruction state, and +execution limits. + +The direct-mapping and access-violation contracts are documented in +[`../README.md`](../README.md). All account references must be released before a +`TransactionContext` is deconstructed. diff --git a/solana/transaction-context/src/instruction.rs b/solana/transaction-context/src/instruction.rs index c90ab5b0..35a4d3c8 100644 --- a/solana/transaction-context/src/instruction.rs +++ b/solana/transaction-context/src/instruction.rs @@ -186,8 +186,7 @@ impl<'a> InstructionContext<'a, '_> { pub fn get_program_key(&self) -> Result<&'a Pubkey, InstructionError> { self.get_index_of_program_account_in_transaction() .and_then(|index_in_transaction| { - self.transaction_context - .get_key_of_account_at_index(index_in_transaction) + self.transaction_context.get_key_of_account_at_index(index_in_transaction) }) } @@ -195,9 +194,7 @@ impl<'a> InstructionContext<'a, '_> { pub fn get_program_owner(&self) -> Result { self.get_index_of_program_account_in_transaction() .and_then(|index_in_transaction| { - self.transaction_context - .accounts - .try_borrow(index_in_transaction) + self.transaction_context.accounts.try_borrow(index_in_transaction) }) .map(|acc| *acc.owner()) } diff --git a/solana/transaction-context/src/instruction_accounts.rs b/solana/transaction-context/src/instruction_accounts.rs index 00ac4119..4bfbac9b 100644 --- a/solana/transaction-context/src/instruction_accounts.rs +++ b/solana/transaction-context/src/instruction_accounts.rs @@ -3,7 +3,7 @@ use { IndexOfAccount, MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION, transaction::TransactionContext, transaction_accounts::AccountRefMut, }, - solana_account::{ReadableAccount, WritableAccount}, + solana_account::{CoWAccount, ReadableAccount, WritableAccount}, solana_instruction::error::InstructionError, solana_pubkey::Pubkey, }; @@ -133,9 +133,7 @@ impl BorrowedInstructionAccount<'_, '_> { } let lamports_balance = (lamports as i128).saturating_sub(old_lamports as i128); - self.transaction_context - .accounts - .add_lamports_delta(lamports_balance)?; + self.transaction_context.accounts.add_lamports_delta(lamports_balance)?; self.touch()?; self.account.set_lamports(lamports); @@ -225,26 +223,19 @@ impl BorrowedInstructionAccount<'_, '_> { Ok(()) } - /// Returns whether the underlying AccountSharedData is shared. + /// Returns whether account data must be mapped through the CoW handler. /// - /// The data is shared if the account has been loaded from the accounts database and has never - /// been written to. Writing to an account unshares it. - /// - /// During account serialization, if an account is shared it'll get mapped as CoW, else it'll - /// get mapped directly as writable. + /// Owned shared buffers and borrowed account images both need first-write + /// translation before the VM can mutate them. pub fn is_shared(&self) -> bool { - self.account.is_shared() + self.account.is_shared() || matches!(self.account.cow(), CoWAccount::Borrowed(_)) } fn make_data_mut(&mut self) { - // if the account is still shared, it means this is the first time we're - // about to write into it. Make the account mutable by copying it in a - // buffer with MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION capacity so that if the - // transaction reallocs, we don't have to copy the whole account data a - // second time to fullfill the realloc. - if self.account.is_shared() { - self.account - .reserve(MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION); + // Reserve the maximum per-instruction growth before mutating shared buffers or borrowed + // account images. Borrowed images with enough spare capacity can remain borrowed. + if self.is_shared() { + self.account.reserve(MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION); } } @@ -270,15 +261,14 @@ impl BorrowedInstructionAccount<'_, '_> { // Returns whether or the lamports currently in the account is sufficient for rent exemption should the // data be resized to the given size pub fn is_rent_exempt_at_data_length(&self, data_length: usize) -> bool { - self.transaction_context - .rent - .is_exempt(self.get_lamports(), data_length) + self.transaction_context.rent.is_exempt(self.get_lamports(), data_length) } /// Returns whether this account is executable (transaction wide) #[inline] #[deprecated(since = "2.1.0", note = "Use `get_owner` instead")] pub fn is_executable(&self) -> bool { + #[allow(deprecated)] self.account.executable() } @@ -301,7 +291,7 @@ impl BorrowedInstructionAccount<'_, '_> { return Err(InstructionError::ExecutableModified); } // don't touch the account if the executable flag does not change - #[expect(deprecated)] + #[allow(deprecated)] if self.is_executable() == is_executable { return Ok(()); } @@ -350,13 +340,17 @@ impl BorrowedInstructionAccount<'_, '_> { /// Returns an error if the account data can not be resized to the given length pub fn can_data_be_resized(&self, new_len: usize) -> Result<(), InstructionError> { let old_len = self.get_data().len(); - // Only the owner can change the length of the data - if new_len != old_len && !self.is_owned_by_current_program() { - return Err(InstructionError::AccountDataSizeChanged); + if new_len != old_len { + use solana_account::AccountMode; + if !self.is_owned_by_current_program() { + // Only the owner can change the length of the data + return Err(InstructionError::AccountDataSizeChanged); + } else if self.account.is(AccountMode::Ephemeral) { + // Ephemeral accounts can only be resized with special builtin instruction + return Err(InstructionError::InvalidRealloc); + } } - self.transaction_context - .accounts - .can_data_be_resized(old_len, new_len)?; + self.transaction_context.accounts.can_data_be_resized(old_len, new_len)?; self.can_data_be_changed() } @@ -379,7 +373,7 @@ fn is_zeroed(buf: &[u8]) -> bool { const ZEROS: [u8; ZEROS_LEN] = [0; ZEROS_LEN]; let mut chunks = buf.chunks_exact(ZEROS_LEN); - #[expect(clippy::indexing_slicing)] + #[allow(clippy::indexing_slicing)] { chunks.all(|chunk| chunk == &ZEROS[..]) && chunks.remainder() == &ZEROS[..chunks.remainder().len()] diff --git a/solana/transaction-context/src/lib.rs b/solana/transaction-context/src/lib.rs index cfbe4022..35663868 100644 --- a/solana/transaction-context/src/lib.rs +++ b/solana/transaction-context/src/lib.rs @@ -1,7 +1,7 @@ -#![cfg(feature = "agave-unstable-api")] -//! Data shared between program runtime and built-in programs as well as SBF programs. +#![allow(clippy::disallowed_methods)] #![deny(clippy::indexing_slicing)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] +#![doc = include_str!("../README.md")] pub mod instruction; pub mod instruction_accounts; @@ -17,9 +17,9 @@ pub const MAX_ACCOUNTS_PER_TRANSACTION: usize = 256; pub const MAX_ACCOUNTS_PER_INSTRUCTION: usize = 255; pub const MAX_INSTRUCTION_DATA_LEN: usize = 10 * 1024; pub const MAX_ACCOUNT_DATA_LEN: u64 = 10 * 1024 * 1024; -// Note: With virtual_address_space_adjustments programs can grow accounts -// faster than they intend to, because the AccessViolationHandler might grow -// an account up to MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION at once. +// Note: Direct account-region mapping lets programs grow accounts through the +// AccessViolationHandler, which might grow an account up to +// MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION at once. pub const MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION: i64 = MAX_ACCOUNT_DATA_LEN as i64 * 2; pub const MAX_ACCOUNT_DATA_GROWTH_PER_INSTRUCTION: usize = 10 * 1_024; // Maximum cross-program invocation and instructions per transaction diff --git a/solana/transaction-context/src/transaction.rs b/solana/transaction-context/src/transaction.rs index dbe3c126..462099da 100644 --- a/solana/transaction-context/src/transaction.rs +++ b/solana/transaction-context/src/transaction.rs @@ -1,7 +1,6 @@ use { crate::{ - IndexOfAccount, MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION, MAX_ACCOUNT_DATA_LEN, - MAX_ACCOUNTS_PER_TRANSACTION, + IndexOfAccount, MAX_ACCOUNT_DATA_LEN, MAX_ACCOUNTS_PER_TRANSACTION, MAX_CPI_TRACE_LENGTH, instruction::{InstructionContext, InstructionFrame}, instruction_accounts::InstructionAccount, transaction_accounts::{KeyedAccountSharedData, TransactionAccounts}, @@ -88,32 +87,19 @@ impl<'ix_data> TransactionContext<'ix_data> { let transaction_frame = TransactionFrame { return_data_pubkey: Pubkey::default(), return_data_scratchpad: VmSlice::new(RETURN_DATA_SCRATCHPAD, 0), - cpi_scratchpad: VmSlice::new( - GUEST_INSTRUCTION_DATA_BASE_ADDRESS.saturating_add( - GUEST_REGION_SIZE.saturating_mul(number_of_top_level_instructions as u64), - ), - 0, - ), + cpi_scratchpad: VmSlice::new(0, 0), current_executing_instruction: 0, total_number_of_instructions_in_trace: number_of_top_level_instructions as u16, number_of_cpis_in_trace: 0, number_of_transaction_accounts: transaction_accounts.len() as u16, }; - // We need an extra space for the placeholder, so we avoid relocations. - let mut instruction_trace = - Vec::with_capacity(instruction_trace_capacity.saturating_add(1)); - instruction_trace.resize_with( - number_of_top_level_instructions.saturating_add(1), - InstructionFrame::default, - ); - Self { accounts: Rc::new(TransactionAccounts::new(transaction_accounts)), instruction_stack_capacity, instruction_trace_capacity, instruction_stack: Vec::with_capacity(instruction_stack_capacity), - instruction_trace, + instruction_trace: vec![InstructionFrame::default()], return_data_bytes: Vec::new(), transaction_frame, next_top_level_instruction_index: 0, @@ -181,10 +167,8 @@ impl<'ix_data> TransactionContext<'ix_data> { &self, index_in_trace: usize, ) -> Result, InstructionError> { - let instruction = self - .instruction_trace - .get(index_in_trace) - .ok_or(InstructionError::CallDepth)?; + let instruction = + self.instruction_trace.get(index_in_trace).ok_or(InstructionError::CallDepth)?; // These commands will return a default empty slice if we are retrieving an instruction // that hasn't been configured yet. @@ -220,10 +204,8 @@ impl<'ix_data> TransactionContext<'ix_data> { &self, nesting_level: usize, ) -> Result, InstructionError> { - let index_in_trace = *self - .instruction_stack - .get(nesting_level) - .ok_or(InstructionError::CallDepth)?; + let index_in_trace = + *self.instruction_stack.get(nesting_level).ok_or(InstructionError::CallDepth)?; let instruction_context = self.get_instruction_context_at_index_in_trace(index_in_trace)?; debug_assert_eq!(instruction_context.nesting_level, nesting_level); Ok(instruction_context) @@ -242,10 +224,7 @@ impl<'ix_data> TransactionContext<'ix_data> { /// Returns the index in the instruction trace of the current executing instruction pub fn get_current_instruction_index(&self) -> Result { - self.instruction_stack - .last() - .copied() - .ok_or(InstructionError::CallDepth) + self.instruction_stack.last().copied().ok_or(InstructionError::CallDepth) } /// Returns a view on the current instruction @@ -262,14 +241,8 @@ impl<'ix_data> TransactionContext<'ix_data> { pub fn get_next_instruction_context( &self, ) -> Result, InstructionError> { - let index_in_trace = if self.instruction_stack.is_empty() { - self.next_top_level_instruction_index - } else { - self.instruction_trace - .len() - .checked_sub(1) - .ok_or(InstructionError::CallDepth)? - }; + let index_in_trace = + self.instruction_trace.len().checked_sub(1).ok_or(InstructionError::CallDepth)?; self.get_instruction_context_at_index_in_trace(index_in_trace) } @@ -290,34 +263,28 @@ impl<'ix_data> TransactionContext<'ix_data> { .get_mut(instruction_index) .ok_or(InstructionError::MaxInstructionTraceLengthExceeded)?; - // If we have a parent index, then we are dealing with a CPI. - if let Some(caller_index) = caller_index { - self.transaction_frame.total_number_of_instructions_in_trace = self - .transaction_frame - .total_number_of_instructions_in_trace - .saturating_add(1); + let total_number_of_instructions_in_trace = if let Some(caller_index) = caller_index { instruction.index_of_caller_instruction = caller_index; - let next_ptr = self - .transaction_frame - .cpi_scratchpad - .ptr() - .saturating_add(GUEST_REGION_SIZE); - self.transaction_frame.cpi_scratchpad = VmSlice::new(next_ptr, 0); self.transaction_frame.total_number_of_instructions_in_trace.saturating_add(1) } else { self.transaction_frame.total_number_of_instructions_in_trace }; + self.transaction_frame.cpi_scratchpad = VmSlice::new( + GUEST_INSTRUCTION_DATA_BASE_ADDRESS.saturating_add( + GUEST_REGION_SIZE.saturating_mul(total_number_of_instructions_in_trace as u64), + ), + 0, + ); + instruction.program_account_index_in_tx = program_index; instruction.configure_vm_slices( instruction_index as u64, instruction_accounts.len(), instruction_data.len() as u64, ); - self.deduplication_maps - .push(deduplication_map.into_boxed_slice()); - self.instruction_accounts - .push(instruction_accounts.into_boxed_slice()); + self.deduplication_maps.push(deduplication_map.into_boxed_slice()); + self.instruction_accounts.push(instruction_accounts.into_boxed_slice()); self.instruction_data.push(instruction_data); Ok(()) } @@ -326,9 +293,8 @@ impl<'ix_data> TransactionContext<'ix_data> { fn deduplicate_accounts_for_tests(instruction_accounts: &[InstructionAccount]) -> Vec { let mut dedup_map = vec![u16::MAX; MAX_ACCOUNTS_PER_TRANSACTION]; for (idx, account) in instruction_accounts.iter().enumerate() { - let index_in_instruction = dedup_map - .get_mut(account.index_in_transaction as usize) - .unwrap(); + let index_in_instruction = + dedup_map.get_mut(account.index_in_transaction as usize).unwrap(); if *index_in_instruction == u16::MAX { *index_in_instruction = idx as u16; } @@ -347,7 +313,7 @@ impl<'ix_data> TransactionContext<'ix_data> { let dedup_map = Self::deduplicate_accounts_for_tests(&instruction_accounts); self.configure_instruction_at_index( - self.next_top_level_instruction_index, + self.get_instruction_trace_length(), program_index, instruction_accounts, dedup_map, @@ -385,36 +351,37 @@ impl<'ix_data> TransactionContext<'ix_data> { if !self.instruction_stack.is_empty() && self.accounts.get_lamports_delta() != 0 { return Err(InstructionError::UnbalancedInstruction); } - { - let instruction = self - .instruction_trace - .last_mut() - .ok_or(InstructionError::CallDepth)?; - instruction.nesting_level = nesting_level as u16; + let index_in_trace = self.get_instruction_trace_length(); + if index_in_trace >= self.instruction_trace_capacity { + return Err(InstructionError::MaxInstructionTraceLengthExceeded); } - if self.number_of_called_instructions_in_trace() >= self.instruction_trace_capacity { + let is_cpi = !self.instruction_stack.is_empty(); + if is_cpi + && (self.transaction_frame.number_of_cpis_in_trace as usize >= MAX_CPI_TRACE_LENGTH + || self.transaction_frame.total_number_of_instructions_in_trace as usize + >= self.instruction_trace_capacity) + { return Err(InstructionError::MaxInstructionTraceLengthExceeded); } - let (index_in_trace, current_top_level_instruction) = if self.instruction_stack.is_empty() { + let instruction = self.instruction_trace.last_mut().ok_or(InstructionError::CallDepth)?; + instruction.nesting_level = nesting_level as u16; + + let current_top_level_instruction = if is_cpi { + self.transaction_frame.total_number_of_instructions_in_trace = + self.transaction_frame.total_number_of_instructions_in_trace.saturating_add(1); + self.transaction_frame.number_of_cpis_in_trace = + self.transaction_frame.number_of_cpis_in_trace.saturating_add(1); + self.next_top_level_instruction_index.saturating_sub(1) + } else { let index = self.next_top_level_instruction_index; self.next_top_level_instruction_index = self.next_top_level_instruction_index.saturating_add(1); - (index, index) - } else { - let index = self.get_instruction_trace_length(); - self.transaction_frame.number_of_cpis_in_trace = self - .transaction_frame - .number_of_cpis_in_trace - .saturating_add(1); - self.instruction_trace.push(InstructionFrame::default()); - ( - index, - self.next_top_level_instruction_index.saturating_sub(1), - ) + index }; + self.instruction_trace.push(InstructionFrame::default()); if nesting_level >= self.instruction_stack_capacity { return Err(InstructionError::CallDepth); } @@ -440,22 +407,21 @@ impl<'ix_data> TransactionContext<'ix_data> { } // Verify (before we pop) that the total sum of all lamports in this instruction did not change let detected_an_unbalanced_instruction = - self.get_current_instruction_context() - .and_then(|instruction_context| { - // Verify all executable accounts have no outstanding refs - self.accounts - .try_borrow_mut( - instruction_context.get_index_of_program_account_in_transaction()?, - ) - .map_err(|err| { - if err == InstructionError::AccountBorrowFailed { - InstructionError::AccountBorrowOutstanding - } else { - err - } - })?; - Ok(self.accounts.get_lamports_delta() != 0) - }); + self.get_current_instruction_context().and_then(|instruction_context| { + // Verify all executable accounts have no outstanding refs + self.accounts + .try_borrow_mut( + instruction_context.get_index_of_program_account_in_transaction()?, + ) + .map_err(|err| { + if err == InstructionError::AccountBorrowFailed { + InstructionError::AccountBorrowOutstanding + } else { + err + } + })?; + Ok(self.accounts.get_lamports_delta() != 0) + }); // Always pop, even if we `detected_an_unbalanced_instruction` self.instruction_stack.pop(); if let Some(instr_idx) = self.instruction_stack.last() { @@ -487,20 +453,14 @@ impl<'ix_data> TransactionContext<'ix_data> { // and `return_data_bytes` is being reset to `data` // in the next statement. unsafe { - self.transaction_frame - .return_data_scratchpad - .set_len(data.len() as u64); + self.transaction_frame.return_data_scratchpad.set_len(data.len() as u64); } self.return_data_bytes = data; Ok(()) } /// Returns a new account data write access handler - pub fn access_violation_handler( - &self, - virtual_address_space_adjustments: bool, - account_data_direct_mapping: bool, - ) -> AccessViolationHandler { + pub fn access_violation_handler(&self) -> AccessViolationHandler { let accounts = Rc::clone(&self.accounts); Box::new( move |region: &mut MemoryRegion, @@ -508,6 +468,8 @@ impl<'ix_data> TransactionContext<'ix_data> { access_type: AccessType, vm_addr: u64, len: u64| { + use solana_account::AccountMode; + if access_type == AccessType::Load { return; } @@ -534,35 +496,25 @@ impl<'ix_data> TransactionContext<'ix_data> { return; } - let remaining_allowed_growth = MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION - .saturating_sub(accounts.resize_delta()) - .max(0) as usize; - if requested_length > region.len as usize { - // Realloc immediately here to fit the requested access, - // then later in CPI or deserialization realloc again to the - // account length the program stored in AccountInfo. let old_len = account.data().len(); - let new_len = (address_space_reserved_for_account as usize) - .min(MAX_ACCOUNT_DATA_LEN as usize) - .min(old_len.saturating_add(remaining_allowed_growth)); - // The last two min operations ensure the following: - debug_assert!(accounts.can_data_be_resized(old_len, new_len).is_ok()); - if accounts - .update_accounts_resize_delta(old_len, new_len) - .is_err() + let new_len = requested_length; + if new_len > MAX_ACCOUNT_DATA_LEN as usize + || accounts.can_data_be_resized(old_len, new_len).is_err() + || account.is(AccountMode::Ephemeral) { return; } + if accounts.update_accounts_resize_delta(old_len, new_len).is_err() { + return; + } account.resize(new_len, 0); - region.len = new_len as u64; } - // Potentially unshare / make the account shared data unique (CoW logic). - if virtual_address_space_adjustments && account_data_direct_mapping { - region.host_addr = account.data_as_mut_slice().as_mut_ptr() as u64; - region.writable = true; - } + let data = account.data_as_mut_slice(); + region.host_addr = data.as_mut_ptr() as u64; + region.len = data.len() as u64; + region.writable = true; }, ) } @@ -579,12 +531,10 @@ impl<'ix_data> TransactionContext<'ix_data> { ) } - /// Called instruction are those that the program runtime has already called into. It - /// encompasses instructions under execution (e.g. all nested CPIs are already called) and - /// finished ones. - /// - /// Top level instructions that have not yet been executed aren't considered called. - pub fn number_of_called_instructions_in_trace(&self) -> usize { + /// An active instruction is either one that has already finished execution or that is + /// under execution (e.g. all nested CPIs are active). + /// For ABIv2 only. + pub fn number_of_active_instructions_in_trace(&self) -> usize { self.next_top_level_instruction_index .saturating_add(self.transaction_frame.number_of_cpis_in_trace as usize) } @@ -603,7 +553,6 @@ impl<'ix_data> TransactionContext<'ix_data> { /// Return data at the end of a transaction #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -#[cfg_attr(feature = "wincode", derive(wincode::SchemaRead, wincode::SchemaWrite))] #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct TransactionReturnData { pub program_id: Pubkey, @@ -626,11 +575,9 @@ impl From> for ExecutionRecord { let (accounts, touched_flags, resize_delta) = Rc::try_unwrap(context.accounts) .expect("transaction_context.accounts has unexpected outstanding refs") .take(); - let touched_account_count = touched_flags - .iter() - .fold(0usize, |accumulator, was_touched| { - accumulator.saturating_add(was_touched.get() as usize) - }) as u64; + let touched_account_count = touched_flags.iter().fold(0usize, |accumulator, was_touched| { + accumulator.saturating_add(was_touched.get() as usize) + }) as u64; let return_data = TransactionReturnData { program_id: context.transaction_frame.return_data_pubkey, @@ -817,10 +764,8 @@ mod tests { let mut transaction_context = TransactionContext::new(transaction_accounts, Rent::default(), 20, 20, 3); - let instruction_accounts_1 = vec![ - InstructionAccount::new(0, false, true), - InstructionAccount::new(3, true, false), - ]; + let instruction_accounts_1 = + vec![InstructionAccount::new(0, false, true), InstructionAccount::new(3, true, false)]; transaction_context .configure_top_level_instruction_for_tests( 1, @@ -860,9 +805,8 @@ mod tests { .unwrap(); transaction_context.push().unwrap(); - let first_ix_context = transaction_context - .get_instruction_context_at_index_in_trace(0) - .unwrap(); + let first_ix_context = + transaction_context.get_instruction_context_at_index_in_trace(0).unwrap(); assert_eq!( instruction_accounts_1.as_slice(), first_ix_context.instruction_accounts @@ -873,17 +817,13 @@ mod tests { ); for (idx_in_ix, acc) in instruction_accounts_1.iter().enumerate() { assert_eq!( - *first_ix_context - .dedup_map - .get(acc.index_in_transaction as usize) - .unwrap(), + *first_ix_context.dedup_map.get(acc.index_in_transaction as usize).unwrap(), idx_in_ix as u16 ); } - let second_ix_context = transaction_context - .get_instruction_context_at_index_in_trace(1) - .unwrap(); + let second_ix_context = + transaction_context.get_instruction_context_at_index_in_trace(1).unwrap(); assert_eq!( instruction_accounts_2.as_slice(), second_ix_context.instruction_accounts @@ -894,17 +834,13 @@ mod tests { ); for (idx_in_ix, acc) in instruction_accounts_2.iter().enumerate() { assert_eq!( - *second_ix_context - .dedup_map - .get(acc.index_in_transaction as usize) - .unwrap(), + *second_ix_context.dedup_map.get(acc.index_in_transaction as usize).unwrap(), idx_in_ix as u16 ); } - let third_ix_context = transaction_context - .get_instruction_context_at_index_in_trace(2) - .unwrap(); + let third_ix_context = + transaction_context.get_instruction_context_at_index_in_trace(2).unwrap(); assert_eq!( instruction_accounts_3.as_slice(), third_ix_context.instruction_accounts @@ -916,18 +852,12 @@ mod tests { for (idx_in_ix, acc) in instruction_accounts_3.iter().enumerate() { if idx_in_ix == 3 { assert_eq!( - *third_ix_context - .dedup_map - .get(acc.index_in_transaction as usize) - .unwrap(), + *third_ix_context.dedup_map.get(acc.index_in_transaction as usize).unwrap(), 1 ); } else { assert_eq!( - *third_ix_context - .dedup_map - .get(acc.index_in_transaction as usize) - .unwrap(), + *third_ix_context.dedup_map.get(acc.index_in_transaction as usize).unwrap(), idx_in_ix as u16 ); } @@ -940,9 +870,7 @@ mod tests { let mut transaction_context = TransactionContext::new(transaction_accounts, Rent::default(), 20, 20, 2); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 0 ); @@ -958,42 +886,20 @@ mod tests { ) .unwrap(); - // Instruction #1 - transaction_context - .configure_instruction_at_index( - 1, - 0, - vec![InstructionAccount::new(1, false, false)], - vec![0; MAX_ACCOUNTS_PER_TRANSACTION], - Vec::new().into(), - None, - ) - .unwrap(); - // Executing instruction #0 transaction_context.push().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, + transaction_context.transaction_frame.current_executing_instruction, 0 ); - assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), - 1 - ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 2 ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 0 ); @@ -1006,7 +912,7 @@ mod tests { 0, ); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 1 ); @@ -1021,26 +927,20 @@ mod tests { transaction_context.push().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 2 + transaction_context.transaction_frame.current_executing_instruction, + 1, ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 3 ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 1 ); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 2 ); @@ -1060,16 +960,12 @@ mod tests { transaction_context.push().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 3 + transaction_context.transaction_frame.current_executing_instruction, + 2 ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 4 ); @@ -1079,40 +975,32 @@ mod tests { ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 2 ); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 3 ); // Return from nested CPI transaction_context.pop().unwrap(); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 3 ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 4 ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 2, ); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 2 + transaction_context.transaction_frame.current_executing_instruction, + 1 ); // A second nested CPI @@ -1126,16 +1014,12 @@ mod tests { transaction_context.push().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 4 + transaction_context.transaction_frame.current_executing_instruction, + 3 ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 5 ); @@ -1144,13 +1028,11 @@ mod tests { GUEST_INSTRUCTION_DATA_BASE_ADDRESS.saturating_add(GUEST_REGION_SIZE.saturating_mul(5)) ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 3 ); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 4 ); @@ -1158,16 +1040,12 @@ mod tests { transaction_context.pop().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 2 + transaction_context.transaction_frame.current_executing_instruction, + 1 ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 5 ); @@ -1177,30 +1055,24 @@ mod tests { ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 3 ); // Return from first CPI transaction_context.pop().unwrap(); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 4 ); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, + transaction_context.transaction_frame.current_executing_instruction, 0 ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 5 ); @@ -1210,25 +1082,28 @@ mod tests { ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 3, ); // Let's go to Instruction #1 (top level) transaction_context.pop().unwrap(); + + // Instruction #1 + transaction_context + .configure_top_level_instruction_for_tests( + 0, + vec![InstructionAccount::new(1, false, false)], + Vec::new(), + ) + .unwrap(); transaction_context.push().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 1, + transaction_context.transaction_frame.current_executing_instruction, + 4, ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 3 ); @@ -1244,16 +1119,12 @@ mod tests { transaction_context.push().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, + transaction_context.transaction_frame.current_executing_instruction, 5, ); assert_eq!( - transaction_context - .transaction_frame - .total_number_of_instructions_in_trace, + transaction_context.transaction_frame.total_number_of_instructions_in_trace, 6 ); @@ -1262,29 +1133,23 @@ mod tests { GUEST_INSTRUCTION_DATA_BASE_ADDRESS.saturating_add(GUEST_REGION_SIZE.saturating_mul(6)) ); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 4 ); assert_eq!( - transaction_context.number_of_called_instructions_in_trace(), + transaction_context.number_of_active_instructions_in_trace(), 6 ); // Return from CPI transaction_context.pop().unwrap(); assert_eq!( - transaction_context - .transaction_frame - .number_of_cpis_in_trace, + transaction_context.transaction_frame.number_of_cpis_in_trace, 4 ); assert_eq!( - transaction_context - .transaction_frame - .current_executing_instruction, - 1, + transaction_context.transaction_frame.current_executing_instruction, + 4, ); transaction_context.pop().unwrap(); @@ -1310,6 +1175,12 @@ mod tests { None, ) .unwrap(); + transaction_context.push().unwrap(); + assert_eq!( + transaction_context.get_current_instruction_index().unwrap(), + 0 + ); + transaction_context.pop().unwrap(); // Second top-level instruction transaction_context @@ -1325,15 +1196,6 @@ mod tests { None, ) .unwrap(); - - transaction_context.push().unwrap(); - assert_eq!( - transaction_context.get_current_instruction_index().unwrap(), - 0 - ); - - transaction_context.pop().unwrap(); - transaction_context.push().unwrap(); assert_eq!( transaction_context.get_current_instruction_index().unwrap(), diff --git a/solana/transaction-context/src/transaction_accounts.rs b/solana/transaction-context/src/transaction_accounts.rs index a2dc8d89..957dcf27 100644 --- a/solana/transaction-context/src/transaction_accounts.rs +++ b/solana/transaction-context/src/transaction_accounts.rs @@ -1,232 +1,64 @@ -#[cfg(feature = "dev-context-only-utils")] -use qualifier_attr::qualifiers; use { - crate::{ - IndexOfAccount, MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION, MAX_ACCOUNT_DATA_LEN, - vm_addresses::{GUEST_ACCOUNT_PAYLOAD_BASE_ADDRESS, GUEST_REGION_SIZE}, - vm_slice::VmSlice, - }, - solana_account::{AccountSharedData, ReadableAccount, WritableAccount}, + crate::{IndexOfAccount, MAX_ACCOUNT_DATA_GROWTH_PER_TRANSACTION, MAX_ACCOUNT_DATA_LEN}, + solana_account::AccountSharedData, solana_instruction::error::InstructionError, solana_pubkey::Pubkey, std::{ cell::{Cell, UnsafeCell}, ops::{Deref, DerefMut}, - ptr, - sync::Arc, }, }; -/// This struct is shared with programs. Do not alter its fields. -#[repr(C)] -#[derive(Debug, PartialEq)] -struct AccountSharedFields { - key: Pubkey, - owner: Pubkey, - lamports: u64, - // The payload is going to be filled with the guest virtual address of the account payload - // vector. - payload: VmSlice, -} - -#[derive(Debug, PartialEq)] -#[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] -struct AccountPrivateFields { - rent_epoch: u64, - executable: bool, - payload: Arc>, -} - -#[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] -impl AccountPrivateFields { - fn payload_len(&self) -> usize { - self.payload.len() - } -} - #[derive(Debug, PartialEq)] #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] pub struct TransactionAccountView<'a> { - abi_account: &'a AccountSharedFields, - private_fields: &'a AccountPrivateFields, + account: &'a AccountSharedData, } #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] -impl ReadableAccount for TransactionAccountView<'_> { - fn lamports(&self) -> u64 { - self.abi_account.lamports - } - - fn data(&self) -> &[u8] { - self.private_fields.payload.as_slice() - } - - fn owner(&self) -> &Pubkey { - &self.abi_account.owner - } - - fn executable(&self) -> bool { - self.private_fields.executable - } - - fn rent_epoch(&self) -> u64 { - self.private_fields.rent_epoch +impl Deref for TransactionAccountView<'_> { + type Target = AccountSharedData; + fn deref(&self) -> &Self::Target { + self.account } } #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] impl PartialEq for TransactionAccountView<'_> { fn eq(&self, other: &AccountSharedData) -> bool { - other.lamports() == self.lamports() - && other.data() == self.data() - && other.owner() == self.owner() - && other.executable() == self.executable() - && other.rent_epoch() == self.rent_epoch() + self.account == other } } #[derive(Debug)] #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] pub struct TransactionAccountViewMut<'a> { - abi_account: &'a mut AccountSharedFields, - private_fields: &'a mut AccountPrivateFields, + account: &'a mut AccountSharedData, } #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] impl TransactionAccountViewMut<'_> { - fn data_mut(&mut self) -> &mut Vec { - Arc::make_mut(&mut self.private_fields.payload) - } - - pub(crate) fn resize(&mut self, new_len: usize, value: u8) { - self.data_mut().resize(new_len, value); - // SAFETY: We are synchronizing the lengths. - unsafe { - self.abi_account.payload.set_len(new_len as u64); - } - } - - #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))] - pub(crate) fn set_data_from_slice(&mut self, new_data: &[u8]) { - // If the buffer isn't shared, we're going to memcpy in place. - let Some(data) = Arc::get_mut(&mut self.private_fields.payload) else { - // If the buffer is shared, the cheapest thing to do is to clone the - // incoming slice and replace the buffer. - self.private_fields.payload = Arc::new(new_data.to_vec()); - // SAFETY: We are synchronizing the lengths. - unsafe { - self.abi_account.payload.set_len(new_data.len() as u64); - } - return; - }; - - let new_len = new_data.len(); - - // Reserve additional capacity if needed. Here we make the assumption - // that growing the current buffer is cheaper than doing a whole new - // allocation to make `new_data` owned. - // - // This assumption holds true during CPI, especially when the account - // size doesn't change but the account is only changed in place. And - // it's also true when the account is grown by a small margin (the - // realloc limit is quite low), in which case the allocator can just - // update the allocation metadata without moving. - // - // Shrinking and copying in place is always faster than making - // `new_data` owned, since shrinking boils down to updating the Vec's - // length. - - data.reserve(new_len.saturating_sub(data.len())); - - // Safety: - // We just reserved enough capacity. We set data::len to 0 to avoid - // possible UB on panic (dropping uninitialized elements), do the copy, - // finally set the new length once everything is initialized. - unsafe { - data.set_len(0); - ptr::copy_nonoverlapping(new_data.as_ptr(), data.as_mut_ptr(), new_len); - data.set_len(new_len); - self.abi_account.payload.set_len(new_len as u64); - }; - } - - pub(crate) fn extend_from_slice(&mut self, data: &[u8]) { - self.data_mut().extend_from_slice(data); - // SAFETY: We are synchronizing the lengths. - unsafe { - self.abi_account - .payload - .set_len(self.private_fields.payload_len() as u64); - } - } - pub(crate) fn reserve(&mut self, additional: usize) { - if let Some(data) = Arc::get_mut(&mut self.private_fields.payload) { - data.reserve(additional) - } else { - let mut data = - Vec::with_capacity(self.private_fields.payload_len().saturating_add(additional)); - data.extend_from_slice(self.private_fields.payload.as_slice()); - self.private_fields.payload = Arc::new(data); - } - } - - #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))] - pub(crate) fn is_shared(&self) -> bool { - Arc::strong_count(&self.private_fields.payload) > 1 + self.account.cow_mut().reserve(additional); } } #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] -impl ReadableAccount for TransactionAccountViewMut<'_> { - fn lamports(&self) -> u64 { - self.abi_account.lamports - } - - fn data(&self) -> &[u8] { - self.private_fields.payload.as_slice() - } - - fn owner(&self) -> &Pubkey { - &self.abi_account.owner - } - - fn executable(&self) -> bool { - self.private_fields.executable - } - - fn rent_epoch(&self) -> u64 { - self.private_fields.rent_epoch +impl Deref for TransactionAccountViewMut<'_> { + type Target = AccountSharedData; + fn deref(&self) -> &Self::Target { + self.account } } #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] -impl WritableAccount for TransactionAccountViewMut<'_> { - fn set_lamports(&mut self, lamports: u64) { - self.abi_account.lamports = lamports; - } - - fn data_as_mut_slice(&mut self) -> &mut [u8] { - Arc::make_mut(&mut self.private_fields.payload).as_mut_slice() - } - - fn set_owner(&mut self, owner: Pubkey) { - self.abi_account.owner = owner; - } - - fn copy_into_owner_from_slice(&mut self, source: &[u8]) { - self.abi_account.owner.as_mut().copy_from_slice(source); - } - - fn set_executable(&mut self, executable: bool) { - self.private_fields.executable = executable; - } - - fn set_rent_epoch(&mut self, epoch: u64) { - self.private_fields.rent_epoch = epoch; +impl DerefMut for TransactionAccountViewMut<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.account } } +// /// An account key and the matching account #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] pub type KeyedAccountSharedData = (Pubkey, AccountSharedData); @@ -237,8 +69,7 @@ pub(crate) type DeconstructedTransactionAccounts = #[derive(Debug)] #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] pub struct TransactionAccounts { - shared_account_fields: Box<[UnsafeCell]>, - private_account_fields: Box<[UnsafeCell]>, + accounts: Box<[UnsafeCell]>, borrow_counters: Box<[BorrowCounter]>, touched_flags: Box<[Cell]>, resize_delta: Cell, @@ -250,36 +81,11 @@ impl TransactionAccounts { pub(crate) fn new(accounts: Vec) -> TransactionAccounts { let touched_flags = vec![Cell::new(false); accounts.len()].into_boxed_slice(); let borrow_counters = vec![BorrowCounter::default(); accounts.len()].into_boxed_slice(); - let (shared_accounts, private_fields) = accounts - .into_iter() - .enumerate() - .map(|(idx, item)| { - ( - UnsafeCell::new(AccountSharedFields { - key: item.0, - owner: *item.1.owner(), - lamports: item.1.lamports(), - payload: VmSlice::new( - GUEST_ACCOUNT_PAYLOAD_BASE_ADDRESS - .saturating_add(GUEST_REGION_SIZE.saturating_mul(idx as u64)), - item.1.data().len() as u64, - ), - }), - UnsafeCell::new(AccountPrivateFields { - rent_epoch: item.1.rent_epoch(), - executable: item.1.executable(), - payload: item.1.data_clone(), - }), - ) - }) - .collect::<( - Vec>, - Vec>, - )>(); + let accounts = + accounts.into_iter().map(UnsafeCell::new).collect::>().into_boxed_slice(); TransactionAccounts { - shared_account_fields: shared_accounts.into_boxed_slice(), - private_account_fields: private_fields.into_boxed_slice(), + accounts, borrow_counters, touched_flags, resize_delta: Cell::new(0), @@ -288,7 +94,7 @@ impl TransactionAccounts { } pub(crate) fn len(&self) -> usize { - self.shared_account_fields.len() + self.accounts.len() } pub fn touch(&self, index: IndexOfAccount) -> Result<(), InstructionError> { @@ -330,8 +136,7 @@ impl TransactionAccounts { Ok(()) } - #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))] - pub(crate) fn try_borrow_mut( + pub fn try_borrow_mut( &self, index: IndexOfAccount, ) -> Result, InstructionError> { @@ -344,31 +149,11 @@ impl TransactionAccounts { // SAFETY: The borrow counter guarantees this is the only mutable borrow of this account. // The unwrap is safe because accounts.len() == borrow_counters.len(), so the missing // account error should have been returned above. - let svm_account = unsafe { - &mut *self - .shared_account_fields - .get(index as usize) - .unwrap() - .get() - }; - - let private_fields = unsafe { - &mut *self - .private_account_fields - .get(index as usize) - .unwrap() - .get() - }; - let account = TransactionAccountViewMut { - abi_account: svm_account, - private_fields, + account: unsafe { &mut (*self.accounts.get(index as usize).unwrap().get()).1 }, }; - Ok(AccountRefMut { - account, - borrow_counter, - }) + Ok(AccountRefMut { account, borrow_counter }) } pub fn try_borrow(&self, index: IndexOfAccount) -> Result, InstructionError> { @@ -381,40 +166,17 @@ impl TransactionAccounts { // SAFETY: The borrow counter guarantees there are no mutable borrow of this account. // The unwrap is safe because accounts.len() == borrow_counters.len(), so the missing // account error should have been returned above. - let svm_account = unsafe { - &*self - .shared_account_fields - .get(index as usize) - .unwrap() - .get() - }; + let keyed_account = unsafe { &*self.accounts.get(index as usize).unwrap().get() }; - let private_fields = unsafe { - &*self - .private_account_fields - .get(index as usize) - .unwrap() - .get() - }; + let account = TransactionAccountView { account: &keyed_account.1 }; - let account = TransactionAccountView { - abi_account: svm_account, - private_fields, - }; - - Ok(AccountRef { - account, - borrow_counter, - }) + Ok(AccountRef { account, borrow_counter }) } pub(crate) fn add_lamports_delta(&self, balance: i128) -> Result<(), InstructionError> { let delta = self.lamports_delta.get(); - self.lamports_delta.set( - delta - .checked_add(balance) - .ok_or(InstructionError::ArithmeticOverflow)?, - ); + self.lamports_delta + .set(delta.checked_add(balance).ok_or(InstructionError::ArithmeticOverflow)?); Ok(()) } @@ -422,47 +184,18 @@ impl TransactionAccounts { self.lamports_delta.get() } + fn drain_accounts(&mut self) -> Box<[UnsafeCell]> { + debug_assert_eq!(self.accounts.len(), self.borrow_counters.len()); + debug_assert_eq!(self.accounts.len(), self.touched_flags.len()); + std::mem::take(&mut self.accounts) + } + fn deconstruct_into_keyed_account_shared_data(&mut self) -> Vec { - let shared_account_fields = std::mem::take(&mut self.shared_account_fields); - let private_account_fields = std::mem::take(&mut self.private_account_fields); - shared_account_fields - .into_iter() - .zip(private_account_fields) - .map(|(shared_fields_cell, private_fields_cell)| { - let shared_fields = shared_fields_cell.into_inner(); - let private_fields = private_fields_cell.into_inner(); - ( - shared_fields.key, - AccountSharedData::create_from_existing_shared_data( - shared_fields.lamports, - private_fields.payload.clone(), - shared_fields.owner, - private_fields.executable, - private_fields.rent_epoch, - ), - ) - }) - .collect() + self.drain_accounts().into_iter().map(UnsafeCell::into_inner).collect() } pub(crate) fn deconstruct_into_account_shared_data(&mut self) -> Vec { - let shared_account_fields = std::mem::take(&mut self.shared_account_fields); - let private_account_fields = std::mem::take(&mut self.private_account_fields); - shared_account_fields - .into_iter() - .zip(private_account_fields) - .map(|(shared_fields_cell, private_fields_cell)| { - let shared_fields = shared_fields_cell.into_inner(); - let private_fields = private_fields_cell.into_inner(); - AccountSharedData::create_from_existing_shared_data( - shared_fields.lamports, - private_fields.payload.clone(), - shared_fields.owner, - private_fields.executable, - private_fields.rent_epoch, - ) - }) - .collect() + self.drain_accounts().into_iter().map(|cell| cell.into_inner().1).collect() } pub(crate) fn take(mut self) -> DeconstructedTransactionAccounts { @@ -476,20 +209,12 @@ impl TransactionAccounts { pub(crate) fn account_key(&self, index: IndexOfAccount) -> Option<&Pubkey> { // SAFETY: We never modify an account key, so returning a reference to it is safe. - unsafe { - self.shared_account_fields - .get(index as usize) - .map(|acc| &(*acc.get()).key) - } + unsafe { self.accounts.get(index as usize).map(|acc| &(*acc.get()).0) } } pub(crate) fn account_keys_iter(&self) -> impl Iterator { // SAFETY: We never modify account keys, so returning an immutable reference to them is safe. - unsafe { - self.shared_account_fields - .iter() - .map(|item| &(*item.get()).key) - } + unsafe { self.accounts.iter().map(|item| &(*item.get()).0) } } } @@ -578,13 +303,6 @@ pub struct AccountRefMut<'a> { #[cfg(not(any(target_arch = "bpf", target_arch = "sbf")))] impl Drop for AccountRefMut<'_> { fn drop(&mut self) { - // SAFETY: We are synchronizing the lengths. - unsafe { - self.account - .abi_account - .payload - .set_len(self.account.private_fields.payload_len() as u64); - } self.borrow_counter.release_borrow_mut(); } } @@ -607,8 +325,13 @@ impl DerefMut for AccountRefMut<'_> { #[cfg(all(test, not(target_arch = "sbf"), not(target_arch = "bpf")))] mod tests { use { - crate::transaction_accounts::TransactionAccounts, solana_account::AccountSharedData, - solana_instruction::error::InstructionError, solana_pubkey::Pubkey, + crate::transaction_accounts::TransactionAccounts, + solana_account::{ + AccountBuilder, AccountFieldPatch, AccountMode, AccountSharedData, DirtyMarkers, + ReadableAccount, StateFlags, WritableAccount, + }, + solana_instruction::error::InstructionError, + solana_pubkey::Pubkey, }; #[test] @@ -735,4 +458,72 @@ mod tests { } } } + + #[test] + fn preserves_account_shared_data_on_deconstruct() { + let key = Pubkey::new_unique(); + let owner = Pubkey::new_unique(); + let mut account = AccountBuilder::default() + .lamports(23) + .data(vec![1, 2, 3]) + .owner(owner) + .mode(AccountMode::ReadOnly) + .slot(41) + .executable(true) + .build::(); + + AccountFieldPatch::Mode(AccountMode::Ephemeral).apply(&mut account).unwrap(); + AccountFieldPatch::Slot(42).apply(&mut account).unwrap(); + account.set_flags(StateFlags::EXECUTABLE); + AccountFieldPatch::DataAt { offset: 0, data: vec![4, 5] } + .apply(&mut account) + .unwrap(); + + let expected_markers = account.markers().bits(); + let mut tx_accounts = TransactionAccounts::new(vec![(key, account)]); + let mut accounts = tx_accounts.deconstruct_into_account_shared_data(); + let account = accounts.pop().unwrap(); + + assert!(accounts.is_empty()); + assert!(account.is(AccountMode::Ephemeral)); + assert_eq!(account.slot(), 42); + assert!(account.executable()); + assert_eq!(account.data(), &[4, 5, 3]); + assert_eq!(account.markers().bits(), expected_markers); + } + + #[test] + fn mutable_view_updates_account_shared_data() { + let key = Pubkey::new_unique(); + let owner = Pubkey::new_unique(); + let new_owner = Pubkey::new_unique(); + let tx_accounts = + TransactionAccounts::new(vec![(key, AccountSharedData::new(7, 2, &owner))]); + + { + let mut account = tx_accounts.try_borrow_mut(0).unwrap(); + account.set_lamports(11); + account.set_owner(new_owner); + account.set_executable(true); + account.resize(4, 9); + assert_eq!(account.data(), &[0, 0, 9, 9]); + account.set_data_from_slice(&[1, 2, 3]); + account.extend_from_slice(&[4, 5]); + account.data_as_mut_slice()[0] = 8; + } + + let mut tx_accounts = tx_accounts; + let mut accounts = tx_accounts.deconstruct_into_account_shared_data(); + let account = accounts.pop().unwrap(); + + assert!(accounts.is_empty()); + assert_eq!(account.lamports(), 11); + assert_eq!(account.owner(), &new_owner); + assert!(account.executable()); + assert_eq!(account.data(), &[8, 2, 3, 4, 5]); + assert!(account.markers().contains(DirtyMarkers::LAMPORTS)); + assert!(account.markers().contains(DirtyMarkers::OWNER)); + assert!(account.markers().contains(DirtyMarkers::FLAGS)); + assert!(account.markers().contains(DirtyMarkers::DATA)); + } } diff --git a/solana/transaction-context/src/vm_addresses.rs b/solana/transaction-context/src/vm_addresses.rs index a28a36b5..c2bb03f1 100644 --- a/solana/transaction-context/src/vm_addresses.rs +++ b/solana/transaction-context/src/vm_addresses.rs @@ -1,5 +1,4 @@ pub(crate) const GUEST_REGION_SIZE: u64 = 1 << 32; pub(crate) const RETURN_DATA_SCRATCHPAD: u64 = 7 * GUEST_REGION_SIZE; -pub(crate) const GUEST_ACCOUNT_PAYLOAD_BASE_ADDRESS: u64 = 8 * GUEST_REGION_SIZE; pub(crate) const GUEST_INSTRUCTION_DATA_BASE_ADDRESS: u64 = 264 * GUEST_REGION_SIZE; pub(crate) const GUEST_INSTRUCTION_ACCOUNT_BASE_ADDRESS: u64 = 328 * GUEST_REGION_SIZE; diff --git a/solana/transaction-context/src/vm_slice.rs b/solana/transaction-context/src/vm_slice.rs index b973c5c2..a98566c5 100644 --- a/solana/transaction-context/src/vm_slice.rs +++ b/solana/transaction-context/src/vm_slice.rs @@ -41,8 +41,7 @@ impl VmSlice { } pub fn end(&self) -> u64 { - self.ptr() - .saturating_add(self.len().saturating_mul(size_of::() as u64)) + self.ptr().saturating_add(self.len().saturating_mul(size_of::() as u64)) } /// # Safety