introduce SerializedAccountMetadata (#32644)

* bpf_loader: move computing original account lengths inside serialize_paramters_(aligned|unaligned)

This is in preparation of returning more than just the original length

* bpf_loader: deserialize*: take original lens as an iterator instead of a slice

This is in preparation of extracting account lenghts from a larger
context

* bpf_loader: introduce SerializedAccountMetadata

Instead of passing original_account_lengths around as Vec<usize>,
introduce an explicit type that includes the length and soon more.

program-runtime/src/invoke_context.rs

@@ -146,10 +146,15 @@ impl BpfAllocator {
 
 pub struct SyscallContext {
     pub allocator: BpfAllocator,
-    pub orig_account_lengths: Vec<usize>,
+    pub accounts_metadata: Vec<SerializedAccountMetadata>,
     pub trace_log: Vec<[u64; 12]>,
 }
 
+#[derive(Debug, Clone)]
+pub struct SerializedAccountMetadata {
+    pub original_data_len: usize,
+}
+
 pub struct InvokeContext<'a> {
     pub transaction_context: &'a mut TransactionContext,
     rent: Rent,

programs/bpf_loader/src/lib.rs

@@ -8,7 +8,7 @@ use {
     solana_measure::measure::Measure,
     solana_program_runtime::{
         ic_logger_msg, ic_msg,
-        invoke_context::{BpfAllocator, InvokeContext, SyscallContext},
+        invoke_context::{BpfAllocator, InvokeContext, SerializedAccountMetadata, SyscallContext},
         loaded_programs::{
             LoadProgramMetrics, LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET,
         },
@@ -204,7 +204,7 @@ pub fn calculate_heap_cost(heap_size: u64, heap_cost: u64, enable_rounding_fix:
 pub fn create_vm<'a, 'b>(
     program: &'a Executable<RequisiteVerifier, InvokeContext<'b>>,
     regions: Vec<MemoryRegion>,
-    orig_account_lengths: Vec<usize>,
+    accounts_metadata: Vec<SerializedAccountMetadata>,
     invoke_context: &'a mut InvokeContext<'b>,
     stack: &mut AlignedMemory<HOST_ALIGN>,
     heap: &mut AlignedMemory<HOST_ALIGN>,
@@ -238,7 +238,7 @@ pub fn create_vm<'a, 'b>(
     )?;
     invoke_context.set_syscall_context(SyscallContext {
         allocator: BpfAllocator::new(heap_size as u64),
-        orig_account_lengths,
+        accounts_metadata,
         trace_log: Vec::new(),
     })?;
     Ok(EbpfVm::new(
@@ -253,7 +253,7 @@ pub fn create_vm<'a, 'b>(
 /// Create the SBF virtual machine
 #[macro_export]
 macro_rules! create_vm {
-    ($vm:ident, $program:expr, $regions:expr, $orig_account_lengths:expr, $invoke_context:expr $(,)?) => {
+    ($vm:ident, $program:expr, $regions:expr, $accounts_metadata:expr, $invoke_context:expr $(,)?) => {
         let invoke_context = &*$invoke_context;
         let stack_size = $program.get_config().stack_size();
         let heap_size = invoke_context
@@ -282,7 +282,7 @@ macro_rules! create_vm {
             let vm = $crate::create_vm(
                 $program,
                 $regions,
-                $orig_account_lengths,
+                $accounts_metadata,
                 $invoke_context,
                 &mut stack,
                 &mut heap,
@@ -295,7 +295,7 @@ macro_rules! create_vm {
 
 #[macro_export]
 macro_rules! mock_create_vm {
-    ($vm:ident, $additional_regions:expr, $orig_account_lengths:expr, $invoke_context:expr $(,)?) => {
+    ($vm:ident, $additional_regions:expr, $accounts_metadata:expr, $invoke_context:expr $(,)?) => {
         let loader = std::sync::Arc::new(BuiltinProgram::new_loader(
             solana_rbpf::vm::Config::default(),
         ));
@@ -315,7 +315,7 @@ macro_rules! mock_create_vm {
             $vm,
             &verified_executable,
             $additional_regions,
-            $orig_account_lengths,
+            $accounts_metadata,
             $invoke_context,
         );
     };
@@ -1500,7 +1500,7 @@ fn execute<'a, 'b: 'a>(
         .is_active(&bpf_account_data_direct_mapping::id());
 
     let mut serialize_time = Measure::start("serialize");
-    let (parameter_bytes, regions, account_lengths) = serialization::serialize_parameters(
+    let (parameter_bytes, regions, accounts_metadata) = serialization::serialize_parameters(
         invoke_context.transaction_context,
         instruction_context,
         invoke_context
@@ -1523,7 +1523,7 @@ fn execute<'a, 'b: 'a>(
     let mut execute_time;
     let execution_result = {
         let compute_meter_prev = invoke_context.get_remaining();
-        create_vm!(vm, executable, regions, account_lengths, invoke_context,);
+        create_vm!(vm, executable, regions, accounts_metadata, invoke_context,);
         let mut vm = match vm {
             Ok(info) => info,
             Err(e) => {
@@ -1604,7 +1604,7 @@ fn execute<'a, 'b: 'a>(
                 .get_current_instruction_context()?,
             copy_account_data,
             parameter_bytes,
-            &invoke_context.get_syscall_context()?.orig_account_lengths,
+            &invoke_context.get_syscall_context()?.accounts_metadata,
         )
     }
 

programs/bpf_loader/src/serialization.rs

@@ -2,6 +2,7 @@
 
 use {
     byteorder::{ByteOrder, LittleEndian},
+    solana_program_runtime::invoke_context::SerializedAccountMetadata,
     solana_rbpf::{
         aligned_memory::{AlignedMemory, Pod},
         ebpf::{HOST_ALIGN, MM_INPUT_START},
@@ -175,54 +176,63 @@ pub fn serialize_parameters(
     instruction_context: &InstructionContext,
     should_cap_ix_accounts: bool,
     copy_account_data: bool,
-) -> Result<(AlignedMemory<HOST_ALIGN>, Vec<MemoryRegion>, Vec<usize>), InstructionError> {
+) -> Result<
+    (
+        AlignedMemory<HOST_ALIGN>,
+        Vec<MemoryRegion>,
+        Vec<SerializedAccountMetadata>,
+    ),
+    InstructionError,
+> {
     let num_ix_accounts = instruction_context.get_number_of_instruction_accounts();
     if should_cap_ix_accounts && num_ix_accounts > MAX_INSTRUCTION_ACCOUNTS as IndexOfAccount {
         return Err(InstructionError::MaxAccountsExceeded);
     }
-    let is_loader_deprecated = *instruction_context
-        .try_borrow_last_program_account(transaction_context)?
-        .get_owner()
-        == bpf_loader_deprecated::id();
 
-    let num_accounts = instruction_context.get_number_of_instruction_accounts() as usize;
-    let mut accounts = Vec::with_capacity(num_accounts);
-    let mut account_lengths: Vec<usize> = Vec::with_capacity(num_accounts);
-    for instruction_account_index in 0..instruction_context.get_number_of_instruction_accounts() {
-        if let Some(index) =
-            instruction_context.is_instruction_account_duplicate(instruction_account_index)?
-        {
-            accounts.push(SerializeAccount::Duplicate(index));
-            // unwrap here is safe: if an account is a duplicate, we must have
-            // seen the original already
-            account_lengths.push(*account_lengths.get(index as usize).unwrap());
-        } else {
-            let account = instruction_context
-                .try_borrow_instruction_account(transaction_context, instruction_account_index)?;
-            account_lengths.push(account.get_data().len());
-            accounts.push(SerializeAccount::Account(
-                instruction_account_index,
-                account,
-            ));
-        };
-    }
+    let (program_id, is_loader_deprecated) = {
+        let program_account =
+            instruction_context.try_borrow_last_program_account(transaction_context)?;
+        (
+            *program_account.get_key(),
+            *program_account.get_owner() == bpf_loader_deprecated::id(),
+        )
+    };
+
+    let accounts = (0..instruction_context.get_number_of_instruction_accounts())
+        .map(|instruction_account_index| {
+            if let Some(index) = instruction_context
+                .is_instruction_account_duplicate(instruction_account_index)
+                .unwrap()
+            {
+                SerializeAccount::Duplicate(index)
+            } else {
+                let account = instruction_context
+                    .try_borrow_instruction_account(transaction_context, instruction_account_index)
+                    .unwrap();
+                SerializeAccount::Account(instruction_account_index, account)
+            }
+        })
+        // fun fact: jemalloc is good at caching tiny allocations like this one,
+        // so collecting here is actually faster than passing the iterator
+        // around, since the iterator does the work to produce its items each
+        // time it's iterated on.
+        .collect::<Vec<_>>();
 
     if is_loader_deprecated {
         serialize_parameters_unaligned(
-            transaction_context,
-            instruction_context,
             accounts,
+            instruction_context.get_instruction_data(),
+            &program_id,
             copy_account_data,
         )
     } else {
         serialize_parameters_aligned(
-            transaction_context,
-            instruction_context,
             accounts,
+            instruction_context.get_instruction_data(),
+            &program_id,
             copy_account_data,
         )
     }
-    .map(|(buffer, regions)| (buffer, regions, account_lengths))
 }
 
 pub fn deserialize_parameters(
@@ -230,12 +240,13 @@ pub fn deserialize_parameters(
     instruction_context: &InstructionContext,
     copy_account_data: bool,
     buffer: &[u8],
-    account_lengths: &[usize],
+    accounts_metadata: &[SerializedAccountMetadata],
 ) -> Result<(), InstructionError> {
     let is_loader_deprecated = *instruction_context
         .try_borrow_last_program_account(transaction_context)?
         .get_owner()
         == bpf_loader_deprecated::id();
+    let account_lengths = accounts_metadata.iter().map(|a| a.original_data_len);
     if is_loader_deprecated {
         deserialize_parameters_unaligned(
             transaction_context,
@@ -256,11 +267,18 @@ pub fn deserialize_parameters(
 }
 
 fn serialize_parameters_unaligned(
-    transaction_context: &TransactionContext,
-    instruction_context: &InstructionContext,
     accounts: Vec<SerializeAccount>,
+    instruction_data: &[u8],
+    program_id: &Pubkey,
     copy_account_data: bool,
-) -> Result<(AlignedMemory<HOST_ALIGN>, Vec<MemoryRegion>), InstructionError> {
+) -> Result<
+    (
+        AlignedMemory<HOST_ALIGN>,
+        Vec<MemoryRegion>,
+        Vec<SerializedAccountMetadata>,
+    ),
+    InstructionError,
+> {
     // Calculate size in order to alloc once
     let mut size = size_of::<u64>();
     for account in &accounts {
@@ -283,16 +301,23 @@ fn serialize_parameters_unaligned(
         }
     }
     size += size_of::<u64>() // instruction data len
-         + instruction_context.get_instruction_data().len() // instruction data
+         + instruction_data.len() // instruction data
          + size_of::<Pubkey>(); // program id
 
     let mut s = Serializer::new(size, MM_INPUT_START, false, copy_account_data);
 
+    let mut accounts_metadata: Vec<SerializedAccountMetadata> = Vec::with_capacity(accounts.len());
     s.write::<u64>((accounts.len() as u64).to_le());
     for account in accounts {
         match account {
-            SerializeAccount::Duplicate(position) => s.write(position as u8),
+            SerializeAccount::Duplicate(position) => {
+                accounts_metadata.push(accounts_metadata.get(position as usize).unwrap().clone());
+                s.write(position as u8);
+            }
             SerializeAccount::Account(_, mut account) => {
+                accounts_metadata.push(SerializedAccountMetadata {
+                    original_data_len: account.get_data().len(),
+                });
                 s.write::<u8>(NON_DUP_MARKER);
                 s.write::<u8>(account.is_signer() as u8);
                 s.write::<u8>(account.is_writable() as u8);
@@ -306,28 +331,25 @@ fn serialize_parameters_unaligned(
             }
         };
     }
-    s.write::<u64>((instruction_context.get_instruction_data().len() as u64).to_le());
-    s.write_all(instruction_context.get_instruction_data());
-    s.write_all(
-        instruction_context
-            .try_borrow_last_program_account(transaction_context)?
-            .get_key()
-            .as_ref(),
-    );
+    s.write::<u64>((instruction_data.len() as u64).to_le());
+    s.write_all(instruction_data);
+    s.write_all(program_id.as_ref());
 
-    Ok(s.finish())
+    let (mem, regions) = s.finish();
+    Ok((mem, regions, accounts_metadata))
 }
 
-pub fn deserialize_parameters_unaligned(
+pub fn deserialize_parameters_unaligned<I: IntoIterator<Item = usize>>(
     transaction_context: &TransactionContext,
     instruction_context: &InstructionContext,
     copy_account_data: bool,
     buffer: &[u8],
-    account_lengths: &[usize],
+    account_lengths: I,
 ) -> Result<(), InstructionError> {
     let mut start = size_of::<u64>(); // number of accounts
-    for (instruction_account_index, pre_len) in
-        (0..instruction_context.get_number_of_instruction_accounts()).zip(account_lengths.iter())
+    for (instruction_account_index, pre_len) in (0..instruction_context
+        .get_number_of_instruction_accounts())
+        .zip(account_lengths.into_iter())
     {
         let duplicate =
             instruction_context.is_instruction_account_duplicate(instruction_account_index)?;
@@ -372,11 +394,19 @@ pub fn deserialize_parameters_unaligned(
 }
 
 fn serialize_parameters_aligned(
-    transaction_context: &TransactionContext,
-    instruction_context: &InstructionContext,
     accounts: Vec<SerializeAccount>,
+    instruction_data: &[u8],
+    program_id: &Pubkey,
     copy_account_data: bool,
-) -> Result<(AlignedMemory<HOST_ALIGN>, Vec<MemoryRegion>), InstructionError> {
+) -> Result<
+    (
+        AlignedMemory<HOST_ALIGN>,
+        Vec<MemoryRegion>,
+        Vec<SerializedAccountMetadata>,
+    ),
+    InstructionError,
+> {
+    let mut account_lengths = Vec::with_capacity(accounts.len());
     // Calculate size in order to alloc once
     let mut size = size_of::<u64>();
     for account in &accounts {
@@ -404,7 +434,7 @@ fn serialize_parameters_aligned(
         }
     }
     size += size_of::<u64>() // data len
-    + instruction_context.get_instruction_data().len()
+    + instruction_data.len()
     + size_of::<Pubkey>(); // program id;
 
     let mut s = Serializer::new(size, MM_INPUT_START, true, copy_account_data);
@@ -414,6 +444,9 @@ fn serialize_parameters_aligned(
     for account in accounts {
         match account {
             SerializeAccount::Account(_, mut borrowed_account) => {
+                account_lengths.push(SerializedAccountMetadata {
+                    original_data_len: borrowed_account.get_data().len(),
+                });
                 s.write::<u8>(NON_DUP_MARKER);
                 s.write::<u8>(borrowed_account.is_signer() as u8);
                 s.write::<u8>(borrowed_account.is_writable() as u8);
@@ -427,33 +460,31 @@ fn serialize_parameters_aligned(
                 s.write::<u64>((borrowed_account.get_rent_epoch()).to_le());
             }
             SerializeAccount::Duplicate(position) => {
+                account_lengths.push(account_lengths.get(position as usize).unwrap().clone());
                 s.write::<u8>(position as u8);
                 s.write_all(&[0u8, 0, 0, 0, 0, 0, 0]);
             }
         };
     }
-    s.write::<u64>((instruction_context.get_instruction_data().len() as u64).to_le());
-    s.write_all(instruction_context.get_instruction_data());
-    s.write_all(
-        instruction_context
-            .try_borrow_last_program_account(transaction_context)?
-            .get_key()
-            .as_ref(),
-    );
+    s.write::<u64>((instruction_data.len() as u64).to_le());
+    s.write_all(instruction_data);
+    s.write_all(program_id.as_ref());
 
-    Ok(s.finish())
+    let (mem, regions) = s.finish();
+    Ok((mem, regions, account_lengths))
 }
 
-pub fn deserialize_parameters_aligned(
+pub fn deserialize_parameters_aligned<I: IntoIterator<Item = usize>>(
     transaction_context: &TransactionContext,
     instruction_context: &InstructionContext,
     copy_account_data: bool,
     buffer: &[u8],
-    account_lengths: &[usize],
+    account_lengths: I,
 ) -> Result<(), InstructionError> {
     let mut start = size_of::<u64>(); // number of accounts
-    for (instruction_account_index, pre_len) in
-        (0..instruction_context.get_number_of_instruction_accounts()).zip(account_lengths.iter())
+    for (instruction_account_index, pre_len) in (0..instruction_context
+        .get_number_of_instruction_accounts())
+        .zip(account_lengths.into_iter())
     {
         let duplicate =
             instruction_context.is_instruction_account_duplicate(instruction_account_index)?;
@@ -487,13 +518,13 @@ pub fn deserialize_parameters_aligned(
                     .ok_or(InstructionError::InvalidArgument)?,
             ) as usize;
             start += size_of::<u64>(); // data length
-            if post_len.saturating_sub(*pre_len) > MAX_PERMITTED_DATA_INCREASE
+            if post_len.saturating_sub(pre_len) > MAX_PERMITTED_DATA_INCREASE
                 || post_len > MAX_PERMITTED_DATA_LENGTH as usize
             {
                 return Err(InstructionError::InvalidRealloc);
             }
             // The redundant check helps to avoid the expensive data comparison if we can
-            let alignment_offset = (*pre_len as *const u8).align_offset(BPF_ALIGN_OF_U128);
+            let alignment_offset = (pre_len as *const u8).align_offset(BPF_ALIGN_OF_U128);
             if copy_account_data {
                 let data = buffer
                     .get(start..start + post_len)
@@ -506,7 +537,7 @@ pub fn deserialize_parameters_aligned(
                     Err(err) if borrowed_account.get_data() != data => return Err(err),
                     _ => {}
                 }
-                start += *pre_len; // data
+                start += pre_len; // data
             } else {
                 // See Serializer::write_account() as to why we have this
                 // padding before the realloc region here.
@@ -520,11 +551,11 @@ pub fn deserialize_parameters_aligned(
                 {
                     Ok(()) => {
                         borrowed_account.set_data_length(post_len)?;
-                        let allocated_bytes = post_len.saturating_sub(*pre_len);
+                        let allocated_bytes = post_len.saturating_sub(pre_len);
                         if allocated_bytes > 0 {
                             borrowed_account
                                 .get_data_mut()?
-                                .get_mut(*pre_len..pre_len.saturating_add(allocated_bytes))
+                                .get_mut(pre_len..pre_len.saturating_add(allocated_bytes))
                                 .ok_or(InstructionError::InvalidArgument)?
                                 .copy_from_slice(
                                     data.get(0..allocated_bytes)
@@ -842,7 +873,7 @@ mod tests {
                 .unwrap();
 
             // check serialize_parameters_aligned
-            let (mut serialized, regions, account_lengths) = serialize_parameters(
+            let (mut serialized, regions, accounts_metadata) = serialize_parameters(
                 invoke_context.transaction_context,
                 instruction_context,
                 true,
@@ -907,7 +938,7 @@ mod tests {
                 instruction_context,
                 copy_account_data,
                 serialized.as_slice(),
-                &account_lengths,
+                &accounts_metadata,
             )
             .unwrap();
             for (index_in_transaction, (_key, original_account)) in

programs/bpf_loader/src/syscalls/cpi.rs

@@ -728,10 +728,10 @@ where
 
     // unwrapping here is fine: we're in a syscall and the method below fails
     // only outside syscalls
-    let orig_data_lens = &invoke_context
+    let accounts_metadata = &invoke_context
         .get_syscall_context()
         .unwrap()
-        .orig_account_lengths;
+        .accounts_metadata;
 
     let direct_mapping = invoke_context
         .feature_set
@@ -763,7 +763,7 @@ where
         } else if let Some(caller_account_index) =
             account_info_keys.iter().position(|key| *key == account_key)
         {
-            let original_data_len = *orig_data_lens
+            let original_data_len = accounts_metadata
                 .get(instruction_account.index_in_caller as usize)
                 .ok_or_else(|| {
                     ic_msg!(
@@ -772,7 +772,8 @@ where
                         account_key
                     );
                     Box::new(InstructionError::MissingAccount)
-                })?;
+                })?
+                .original_data_len;
 
             // build the CallerAccount corresponding to this account.
             let caller_account =
@@ -1266,7 +1267,9 @@ mod tests {
     use {
         super::*,
         crate::mock_create_vm,
-        solana_program_runtime::with_mock_invoke_context,
+        solana_program_runtime::{
+            invoke_context::SerializedAccountMetadata, with_mock_invoke_context,
+        },
         solana_rbpf::{
             ebpf::MM_INPUT_START, elf::SBPFVersion, memory_region::MemoryRegion, vm::Config,
         },
@@ -2155,7 +2158,7 @@ mod tests {
         mock_create_vm!(
             _vm,
             Vec::new(),
-            vec![original_data_len],
+            vec![SerializedAccountMetadata { original_data_len }],
             &mut invoke_context
         );