BorrowedAccount: add set_data_from_slice(), make set_data() take owne…

…d values (solana-labs#27836)

* BorrowedAccount: add set_data_from_slice(), make set_data() take owned values

set_data() used to take a slice and would force alloc+copy if the caller
has owned values (eg account creation, account lookup table).

Expose set_data_from_slice() for callers that have slices, and switch
set_data() to taking an owned Vec.

* BorrowAccount: refactor common accounts_update_delta code in helper method

* BorrowedAccount: add extend_from_slice()

This allows avoiding copies appending entries to account lookup tables.

* BorrowedAccount: remove unnecessary ifs around update_accounts_resize_delta

program-runtime/src/invoke_context.rs

@@ -1185,13 +1185,13 @@ mod tests {
                 MockInstruction::NoopFail => return Err(InstructionError::GenericError),
                 MockInstruction::ModifyOwned => instruction_context
                     .try_borrow_instruction_account(transaction_context, 0)?
-                    .set_data(&[1])?,
+                    .set_data_from_slice(&[1])?,
                 MockInstruction::ModifyNotOwned => instruction_context
                     .try_borrow_instruction_account(transaction_context, 1)?
-                    .set_data(&[1])?,
+                    .set_data_from_slice(&[1])?,
                 MockInstruction::ModifyReadonly => instruction_context
                     .try_borrow_instruction_account(transaction_context, 2)?
-                    .set_data(&[1])?,
+                    .set_data_from_slice(&[1])?,
                 MockInstruction::UnbalancedPush => {
                     instruction_context
                         .try_borrow_instruction_account(transaction_context, 0)?
@@ -1239,7 +1239,7 @@ mod tests {
                 }
                 MockInstruction::Resize { new_len } => instruction_context
                     .try_borrow_instruction_account(transaction_context, 0)?
-                    .set_data(&vec![0; new_len as usize])?,
+                    .set_data(vec![0; new_len as usize])?,
             }
         } else {
             return Err(InstructionError::InvalidInstructionData);

program-test/src/lib.rs

@@ -160,7 +160,7 @@ pub fn builtin_process_instruction(
                     .is_ok()
                     && borrowed_account.can_data_be_changed().is_ok()
                 {
-                    borrowed_account.set_data(&account_info.data.borrow())?;
+                    borrowed_account.set_data_from_slice(&account_info.data.borrow())?;
                 }
                 if borrowed_account.get_owner() != account_info.owner {
                     borrowed_account.set_owner(account_info.owner.as_ref())?;
@@ -286,7 +286,9 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
                 .can_data_be_resized(account_info_data.len())
                 .and_then(|_| borrowed_account.can_data_be_changed())
             {
-                Ok(()) => borrowed_account.set_data(&account_info_data).unwrap(),
+                Ok(()) => borrowed_account
+                    .set_data_from_slice(&account_info_data)
+                    .unwrap(),
                 Err(err) if borrowed_account.get_data() != *account_info_data => {
                     panic!("{:?}", err);
                 }

programs/address-lookup-table/src/processor.rs

@@ -304,14 +304,14 @@ impl Processor {
             LOOKUP_TABLE_META_SIZE,
             new_table_addresses_len.saturating_mul(PUBKEY_BYTES),
         )?;
-
         {
-            let mut table_data = lookup_table_account.get_data_mut()?.to_vec();
-            AddressLookupTable::overwrite_meta_data(&mut table_data, lookup_table_meta)?;
+            AddressLookupTable::overwrite_meta_data(
+                lookup_table_account.get_data_mut()?,
+                lookup_table_meta,
+            )?;
             for new_address in new_addresses {
-                table_data.extend_from_slice(new_address.as_ref());
+                lookup_table_account.extend_from_slice(new_address.as_ref())?;
             }
-            lookup_table_account.set_data(&table_data)?;
         }
         drop(lookup_table_account);
 
@@ -462,7 +462,7 @@ impl Processor {
 
         let mut lookup_table_account =
             instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
-        lookup_table_account.set_data(&[])?;
+        lookup_table_account.set_data_length(0)?;
         lookup_table_account.set_lamports(0)?;
 
         Ok(())

programs/bpf_loader/src/serialization.rs

@@ -192,7 +192,7 @@ pub fn deserialize_parameters_unaligned(
                 .can_data_be_resized(data.len())
                 .and_then(|_| borrowed_account.can_data_be_changed())
             {
-                Ok(()) => borrowed_account.set_data(data)?,
+                Ok(()) => borrowed_account.set_data_from_slice(data)?,
                 Err(err) if borrowed_account.get_data() != data => return Err(err),
                 _ => {}
             }
@@ -355,7 +355,7 @@ pub fn deserialize_parameters_aligned(
                 .can_data_be_resized(data.len())
                 .and_then(|_| borrowed_account.can_data_be_changed())
             {
-                Ok(()) => borrowed_account.set_data(data)?,
+                Ok(()) => borrowed_account.set_data_from_slice(data)?,
                 Err(err) if borrowed_account.get_data() != data => return Err(err),
                 _ => {}
             }

programs/bpf_loader/src/syscalls/cpi.rs

@@ -6,6 +6,10 @@ use {
     },
 };
 
+/// Account data as stored in the caller address space during CPI, translated to host memory.
+///
+/// At the start of a CPI, this can be different from the data stored in the
+/// corresponding BorrowedAccount, and needs to be synched.
 struct CallerAccount<'a> {
     lamports: &'a mut u64,
     owner: &'a mut Pubkey,
@@ -663,6 +667,11 @@ where
                 invoke_context,
             )?;
             {
+                // before initiating CPI, the caller may have modified the
+                // account (caller_account). We need to update the corresponding
+                // BorrowedAccount (callee_account) so the callee can see the
+                // changes.
+
                 if callee_account.get_lamports() != *caller_account.lamports {
                     callee_account
                         .set_lamports(*caller_account.lamports)
@@ -674,7 +683,7 @@ where
                     .and_then(|_| callee_account.can_data_be_changed())
                 {
                     Ok(()) => callee_account
-                        .set_data(caller_account.data)
+                        .set_data_from_slice(caller_account.data)
                         .map_err(SyscallError::InstructionError)?,
                     Err(err) if callee_account.get_data() != caller_account.data => {
                         return Err(EbpfError::UserError(BpfError::SyscallError(

runtime/src/bank.rs

@@ -17315,7 +17315,7 @@ pub(crate) mod tests {
             let instruction_context = transaction_context.get_current_instruction_context()?;
             instruction_context
                 .try_borrow_instruction_account(transaction_context, 1)?
-                .set_data(&[0; 40])?;
+                .set_data(vec![0; 40])?;
             Ok(())
         }
 

runtime/src/message_processor.rs

@@ -238,7 +238,7 @@ mod tests {
                     MockSystemInstruction::ChangeData { data } => {
                         instruction_context
                             .try_borrow_instruction_account(transaction_context, 1)?
-                            .set_data(&[data])?;
+                            .set_data(vec![data])?;
                         Ok(())
                     }
                 }
@@ -466,7 +466,7 @@ mod tests {
                             .try_borrow_instruction_account(transaction_context, 2)?;
                         dup_account.checked_sub_lamports(lamports)?;
                         to_account.checked_add_lamports(lamports)?;
-                        dup_account.set_data(&[data])?;
+                        dup_account.set_data(vec![data])?;
                         drop(dup_account);
                         let mut from_account = instruction_context
                             .try_borrow_instruction_account(transaction_context, 0)?;

runtime/src/system_instruction_processor.rs

@@ -108,7 +108,7 @@ fn allocate(
         return Err(SystemError::InvalidAccountDataLength.into());
     }
 
-    account.set_data(&vec![0; space as usize])?;
+    account.set_data_length(space as usize)?;
 
     Ok(())
 }

sdk/src/transaction_context.rs

@@ -826,24 +826,37 @@ impl<'a> BorrowedAccount<'a> {
         Ok(self.account.data_as_mut_slice())
     }
 
-    /// Overwrites the account data and size (transaction wide)
+    /// Overwrites the account data and size (transaction wide).
+    ///
+    /// Call this when you have an owned buffer and want to replace the account
+    /// data with it.
+    ///
+    /// If you have a slice, use set_data_from_slice().
     #[cfg(not(target_os = "solana"))]
-    pub fn set_data(&mut self, data: &[u8]) -> Result<(), InstructionError> {
+    pub fn set_data(&mut self, data: Vec<u8>) -> Result<(), InstructionError> {
         self.can_data_be_resized(data.len())?;
         self.can_data_be_changed()?;
         self.touch()?;
-        if data.len() == self.account.data().len() {
-            self.account.data_as_mut_slice().copy_from_slice(data);
-        } else {
-            let mut accounts_resize_delta = self
-                .transaction_context
-                .accounts_resize_delta
-                .try_borrow_mut()
-                .map_err(|_| InstructionError::GenericError)?;
-            *accounts_resize_delta = accounts_resize_delta
-                .saturating_add((data.len() as i64).saturating_sub(self.get_data().len() as i64));
-            self.account.set_data_from_slice(data);
-        }
+        self.update_accounts_resize_delta(data.len())?;
+        self.account.set_data(data);
+
+        Ok(())
+    }
+
+    /// Overwrites the account data and size (transaction wide).
+    ///
+    /// Call this when you have a slice of data you do not own and want to
+    /// replace the account data with it.
+    ///
+    /// If you have an owned buffer (eg Vec<u8>), use set_data().
+    #[cfg(not(target_os = "solana"))]
+    pub fn set_data_from_slice(&mut self, data: &[u8]) -> Result<(), InstructionError> {
+        self.can_data_be_resized(data.len())?;
+        self.can_data_be_changed()?;
+        self.touch()?;
+        self.update_accounts_resize_delta(data.len())?;
+        self.account.set_data_from_slice(data);
+
         Ok(())
     }
 
@@ -859,13 +872,7 @@ impl<'a> BorrowedAccount<'a> {
             return Ok(());
         }
         self.touch()?;
-        let mut accounts_resize_delta = self
-            .transaction_context
-            .accounts_resize_delta
-            .try_borrow_mut()
-            .map_err(|_| InstructionError::GenericError)?;
-        *accounts_resize_delta = accounts_resize_delta
-            .saturating_add((new_length as i64).saturating_sub(self.get_data().len() as i64));
+        self.update_accounts_resize_delta(new_length)?;
         self.account.data_mut().resize(new_length, 0);
         Ok(())
     }
@@ -880,6 +887,23 @@ impl<'a> BorrowedAccount<'a> {
         }
     }
 
+    /// Appends all elements in a slice to the account
+    #[cfg(not(target_os = "solana"))]
+    pub fn extend_from_slice(&mut self, data: &[u8]) -> Result<(), InstructionError> {
+        let new_len = self.get_data().len().saturating_add(data.len());
+        self.can_data_be_resized(new_len)?;
+        self.can_data_be_changed()?;
+
+        if data.is_empty() {
+            return Ok(());
+        }
+
+        self.touch()?;
+        self.update_accounts_resize_delta(new_len)?;
+        self.account.data_mut().extend_from_slice(data);
+        Ok(())
+    }
+
     /// Deserializes the account data into a state
     #[cfg(not(target_os = "solana"))]
     pub fn get_state<T: serde::de::DeserializeOwned>(&self) -> Result<T, InstructionError> {
@@ -1062,6 +1086,18 @@ impl<'a> BorrowedAccount<'a> {
         }
         Ok(())
     }
+
+    #[cfg(not(target_os = "solana"))]
+    fn update_accounts_resize_delta(&mut self, new_len: usize) -> Result<(), InstructionError> {
+        let mut accounts_resize_delta = self
+            .transaction_context
+            .accounts_resize_delta
+            .try_borrow_mut()
+            .map_err(|_| InstructionError::GenericError)?;
+        *accounts_resize_delta = accounts_resize_delta
+            .saturating_add((new_len as i64).saturating_sub(self.get_data().len() as i64));
+        Ok(())
+    }
 }
 
 /// Everything that needs to be recorded from a TransactionContext after execution