Skip to content

Commit

Permalink
Introduces Result return types to mutable accessors of `BorrowedAcc…
Browse files Browse the repository at this point in the history 
…ount` (solana-labs#25380)

* Introduces result return types to get_data_mut(), set_data() and set_data_length() of BorrowedAccount.

* Introduces result return types to set_owner(), set_lamports() and set_executable() of BorrowedAccount.
  • Loading branch information
@Lichtso @jeffwashington
Lichtso authored and jeffwashington committed Jun 29, 2022
1 parent f0ba452 commit 8b45c83
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 45 deletions.
12 changes: 8 additions & 4 deletions program-runtime/src/invoke_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1404,13 +1404,16 @@ mod tests {
MockInstruction::NoopFail => return Err(InstructionError::GenericError),
MockInstruction::ModifyOwned => instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.set_data(&[1]),
.set_data(&[1])
.unwrap(),
MockInstruction::ModifyNotOwned => instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_data(&[1]),
.set_data(&[1])
.unwrap(),
MockInstruction::ModifyReadonly => instruction_context
.try_borrow_instruction_account(transaction_context, 2)?
.set_data(&[1]),
.set_data(&[1])
.unwrap(),
MockInstruction::ConsumeComputeUnits {
compute_units_to_consume,
desired_result,
Expand All @@ -1424,7 +1427,8 @@ mod tests {
}
MockInstruction::Resize { new_len } => instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.set_data(&vec![0; new_len]),
.set_data(&vec![0; new_len])
.unwrap(),
}
} else {
return Err(InstructionError::InvalidInstructionData);
Expand Down
4 changes: 2 additions & 2 deletions program-test/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ pub fn builtin_process_instruction(
let mut borrowed_account =
instruction_context.try_borrow_account(transaction_context, index_in_instruction)?;
if borrowed_account.is_writable() {
borrowed_account.set_lamports(lamports);
borrowed_account.set_data(&data);
borrowed_account.set_lamports(lamports)?;
borrowed_account.set_data(&data)?;
}
}

Expand Down
12 changes: 6 additions & 6 deletions programs/address-lookup-table/src/processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl Processor {
let mut lookup_table_meta = lookup_table.meta;
lookup_table_meta.authority = None;
AddressLookupTable::overwrite_meta_data(
lookup_table_account.get_data_mut(),
lookup_table_account.get_data_mut()?,
lookup_table_meta,
)?;

Expand Down Expand Up @@ -301,12 +301,12 @@ impl Processor {
)?;

{
let mut table_data = lookup_table_account.get_data_mut().to_vec();
let mut table_data = lookup_table_account.get_data_mut()?.to_vec();
AddressLookupTable::overwrite_meta_data(&mut table_data, lookup_table_meta)?;
for new_address in new_addresses {
table_data.extend_from_slice(new_address.as_ref());
}
lookup_table_account.set_data(&table_data);
lookup_table_account.set_data(&table_data)?;
}
drop(lookup_table_account);

Expand Down Expand Up @@ -380,7 +380,7 @@ impl Processor {
lookup_table_meta.deactivation_slot = clock.slot;

AddressLookupTable::overwrite_meta_data(
lookup_table_account.get_data_mut(),
lookup_table_account.get_data_mut()?,
lookup_table_meta,
)?;

Expand Down Expand Up @@ -468,8 +468,8 @@ 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_lamports(0);
lookup_table_account.set_data(&[])?;
lookup_table_account.set_lamports(0)?;

Ok(())
}
Expand Down
24 changes: 12 additions & 12 deletions programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ fn write_program_data(
let instruction_context = transaction_context.get_current_instruction_context()?;
let mut program =
instruction_context.try_borrow_account(transaction_context, program_account_index)?;
let data = program.get_data_mut();
let data = program.get_data_mut()?;
let write_offset = program_data_offset.saturating_add(bytes.len());
if data.len() < write_offset {
ic_msg!(
Expand Down Expand Up @@ -600,7 +600,7 @@ fn process_loader_upgradeable_instruction(
drop(payer);
let mut buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 3)?;
buffer.set_lamports(0);
buffer.set_lamports(0)?;
}

let mut instruction = system_instruction::create_account(
Expand Down Expand Up @@ -650,7 +650,7 @@ fn process_loader_upgradeable_instruction(
upgrade_authority_address: authority_key,
})?;
let dst_slice = programdata
.get_data_mut()
.get_data_mut()?
.get_mut(
programdata_data_offset
..programdata_data_offset.saturating_add(buffer_data_len),
Expand All @@ -671,7 +671,7 @@ fn process_loader_upgradeable_instruction(
program.set_state(&UpgradeableLoaderState::Program {
programdata_address: programdata_key,
})?;
program.set_executable(true);
program.set_executable(true)?;
drop(program);

if !predrain_buffer {
Expand All @@ -682,7 +682,7 @@ fn process_loader_upgradeable_instruction(
drop(payer);
let mut buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 3)?;
buffer.set_lamports(0);
buffer.set_lamports(0)?;
}

ic_logger_msg!(log_collector, "Deployed program {:?}", new_program_id);
Expand Down Expand Up @@ -833,7 +833,7 @@ fn process_loader_upgradeable_instruction(
upgrade_authority_address: authority_key,
})?;
let dst_slice = programdata
.get_data_mut()
.get_data_mut()?
.get_mut(
programdata_data_offset
..programdata_data_offset.saturating_add(buffer_data_len),
Expand All @@ -848,20 +848,20 @@ fn process_loader_upgradeable_instruction(
dst_slice.copy_from_slice(src_slice);
}
programdata
.get_data_mut()
.get_data_mut()?
.get_mut(programdata_data_offset.saturating_add(buffer_data_len)..)
.ok_or(InstructionError::AccountDataTooSmall)?
.fill(0);

// Fund ProgramData to rent-exemption, spill the rest

let programdata_lamports = programdata.get_lamports();
programdata.set_lamports(programdata_balance_required);
programdata.set_lamports(programdata_balance_required)?;
drop(programdata);

let mut buffer =
instruction_context.try_borrow_instruction_account(transaction_context, 2)?;
buffer.set_lamports(0);
buffer.set_lamports(0)?;
drop(buffer);

let mut spill =
Expand Down Expand Up @@ -962,7 +962,7 @@ fn process_loader_upgradeable_instruction(
match close_account.get_state()? {
UpgradeableLoaderState::Uninitialized => {
let close_lamports = close_account.get_lamports();
close_account.set_lamports(0);
close_account.set_lamports(0)?;
drop(close_account);
let mut recipient_account = instruction_context
.try_borrow_instruction_account(transaction_context, 1)?;
Expand Down Expand Up @@ -1070,7 +1070,7 @@ fn common_close_account(
let mut recipient_account =
instruction_context.try_borrow_instruction_account(transaction_context, 1)?;
recipient_account.checked_add_lamports(close_account.get_lamports())?;
close_account.set_lamports(0);
close_account.set_lamports(0)?;
close_account.set_state(&UpgradeableLoaderState::Uninitialized)?;
Ok(())
}
Expand Down Expand Up @@ -1127,7 +1127,7 @@ fn process_loader_instruction(
let mut program =
instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
invoke_context.update_executor(program.get_key(), executor);
program.set_executable(true);
program.set_executable(true)?;
ic_msg!(invoke_context, "Finalized account {:?}", program.get_key());
}
}
Expand Down
2 changes: 1 addition & 1 deletion programs/config/src/config_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub fn process_instruction(
ic_msg!(invoke_context, "instruction data too large");
return Err(InstructionError::InvalidInstructionData);
}
config_account.get_data_mut()[..data.len()].copy_from_slice(data);
config_account.get_data_mut()?[..data.len()].copy_from_slice(data);
Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16382,7 +16382,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(&[0; 40])?;
Ok(())
}

Expand Down Expand Up @@ -18002,7 +18002,7 @@ pub(crate) mod tests {
// Set data length
instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_data_length(new_size);
.set_data_length(new_size)?;

// set balance
let current_balance = instruction_context
Expand All @@ -18016,14 +18016,14 @@ pub(crate) mod tests {
.checked_sub_lamports(amount)?;
instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_lamports(new_balance);
.set_lamports(new_balance)?;
} else {
instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.checked_add_lamports(amount)?;
instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_lamports(new_balance);
.set_lamports(new_balance)?;
}
Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/src/message_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ mod tests {
MockSystemInstruction::ChangeData { data } => {
instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_data(&[data]);
.set_data(&[data])?;
Ok(())
}
}
Expand Down Expand Up @@ -457,7 +457,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(&[data])?;
drop(dup_account);
let mut from_account = instruction_context
.try_borrow_instruction_account(transaction_context, 0)?;
Expand Down
5 changes: 2 additions & 3 deletions runtime/src/system_instruction_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn allocate(
return Err(SystemError::InvalidAccountDataLength.into());
}

account.set_data(&vec![0; space as usize]);
account.set_data(&vec![0; space as usize])?;

Ok(())
}
Expand All @@ -128,8 +128,7 @@ fn assign(
return Err(InstructionError::MissingRequiredSignature);
}

account.set_owner(&owner.to_bytes());
Ok(())
account.set_owner(&owner.to_bytes())
}

fn allocate_and_assign(
Expand Down
25 changes: 14 additions & 11 deletions sdk/src/transaction_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,9 @@ impl<'a> BorrowedAccount<'a> {
}

/// Assignes the owner of this account (transaction wide)
pub fn set_owner(&mut self, pubkey: &[u8]) {
pub fn set_owner(&mut self, pubkey: &[u8]) -> Result<(), InstructionError> {
self.account.copy_into_owner_from_slice(pubkey);
Ok(())
}

/// Returns the number of lamports of this account (transaction wide)
Expand All @@ -522,8 +523,9 @@ impl<'a> BorrowedAccount<'a> {
}

/// Overwrites the number of lamports of this account (transaction wide)
pub fn set_lamports(&mut self, lamports: u64) {
pub fn set_lamports(&mut self, lamports: u64) -> Result<(), InstructionError> {
self.account.set_lamports(lamports);
Ok(())
}

/// Adds lamports to this account (transaction wide)
Expand All @@ -532,8 +534,7 @@ impl<'a> BorrowedAccount<'a> {
self.get_lamports()
.checked_add(lamports)
.ok_or(LamportsError::ArithmeticOverflow)?,
);
Ok(())
)
}

/// Subtracts lamports from this account (transaction wide)
Expand All @@ -542,8 +543,7 @@ impl<'a> BorrowedAccount<'a> {
self.get_lamports()
.checked_sub(lamports)
.ok_or(LamportsError::ArithmeticUnderflow)?,
);
Ok(())
)
}

/// Returns a read-only slice of the account data (transaction wide)
Expand All @@ -552,24 +552,26 @@ impl<'a> BorrowedAccount<'a> {
}

/// Returns a writable slice of the account data (transaction wide)
pub fn get_data_mut(&mut self) -> &mut [u8] {
self.account.data_as_mut_slice()
pub fn get_data_mut(&mut self) -> Result<&mut [u8], InstructionError> {
Ok(self.account.data_as_mut_slice())
}

/// Overwrites the account data and size (transaction wide)
pub fn set_data(&mut self, data: &[u8]) {
pub fn set_data(&mut self, data: &[u8]) -> Result<(), InstructionError> {
if data.len() == self.account.data().len() {
self.account.data_as_mut_slice().copy_from_slice(data);
} else {
self.account.set_data_from_slice(data);
}
Ok(())
}

/// Resizes the account data (transaction wide)
///
/// Fills it with zeros at the end if is extended or truncates at the end otherwise.
pub fn set_data_length(&mut self, new_len: usize) {
pub fn set_data_length(&mut self, new_len: usize) -> Result<(), InstructionError> {
self.account.data_mut().resize(new_len, 0);
Ok(())
}

/// Deserializes the account data into a state
Expand Down Expand Up @@ -597,8 +599,9 @@ impl<'a> BorrowedAccount<'a> {
}

/// Configures whether this account is executable (transaction wide)
pub fn set_executable(&mut self, is_executable: bool) {
pub fn set_executable(&mut self, is_executable: bool) -> Result<(), InstructionError> {
self.account.set_executable(is_executable);
Ok(())
}

/// Returns the rent epoch of this account (transaction wide)
Expand Down

0 comments on commit 8b45c83

Please sign in to comment.