Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switching to memmove where it makes sense #198

Merged
merged 2 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions programs/mpl-core/src/plugins/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,10 @@ pub fn update_external_plugin_adapter_data<'a, T: DataBlob + SolanaAccount>(
.checked_add(size_diff)
.ok_or(MplCoreError::NumericalOverflow)?;

// SAFETY: `borrow_mut` will always return a valid pointer.
// new_next_plugin_offset is derived from next_plugin_offset and size_diff using
// checked arithmetic, so it will always be less than or equal to account.data_len().
// This will fail and revert state if there is a memory violation.
unsafe {
let base = account.data.borrow_mut().as_mut_ptr();
sol_memmove(
Expand Down Expand Up @@ -621,13 +625,18 @@ pub fn delete_plugin<'a, T: DataBlob>(
.checked_sub(next_plugin_offset)
.ok_or(MplCoreError::NumericalOverflow)?;

//TODO: This is memory intensive, we should use memmove instead probably.
let src = account.data.borrow()[next_plugin_offset..].to_vec();
sol_memcpy(
&mut account.data.borrow_mut()[plugin_offset..],
&src,
data_to_move,
);
// SAFETY: `borrow_mut` will always return a valid pointer.
// plugin_offset is derived from next_plugin_offset and size_diff using
// checked arithmetic, so it will always be less than or equal to account.data_len().
// This will fail and revert state if there is a memory violation.
unsafe {
let base = account.data.borrow_mut().as_mut_ptr();
sol_memmove(
base.add(plugin_offset),
base.add(next_plugin_offset),
data_to_move,
);
}
danenbm marked this conversation as resolved.
Show resolved Hide resolved

header.plugin_registry_offset = new_registry_offset;
header.save(account, asset.get_size())?;
Expand Down Expand Up @@ -699,13 +708,18 @@ pub fn delete_external_plugin_adapter<'a, T: DataBlob>(
.checked_sub(next_plugin_offset)
.ok_or(MplCoreError::NumericalOverflow)?;

//TODO: This is memory intensive, we should use memmove instead probably.
let src = account.data.borrow()[next_plugin_offset..].to_vec();
sol_memcpy(
&mut account.data.borrow_mut()[plugin_offset..],
&src,
data_to_move,
);
// SAFETY: `borrow_mut` will always return a valid pointer.
// plugin_offset is derived from next_plugin_offset and size_diff using
// checked arithmetic, so it will always be less than or equal to account.data_len().
// This will fail and revert state if there is a memory violation.
unsafe {
let base = account.data.borrow_mut().as_mut_ptr();
sol_memmove(
base.add(plugin_offset),
base.add(next_plugin_offset),
data_to_move,
);
}

header.plugin_registry_offset = new_registry_offset;
header.save(account, asset.get_size())?;
Expand Down
22 changes: 13 additions & 9 deletions programs/mpl-core/src/processor/update.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use borsh::{BorshDeserialize, BorshSerialize};
use mpl_utils::assert_signer;
use solana_program::{
account_info::AccountInfo, entrypoint::ProgramResult, msg, program_memory::sol_memcpy,
account_info::AccountInfo, entrypoint::ProgramResult, msg, program_memory::sol_memmove,
};

use crate::{
Expand Down Expand Up @@ -354,16 +354,20 @@ fn process_update<'a, T: DataBlob + SolanaAccount>(
.checked_add(size_diff)
.ok_or(MplCoreError::NumericalOverflow)?;

// //TODO: This is memory intensive, we should use memmove instead probably.
let src = account.data.borrow()[(plugin_offset as usize)..registry_offset].to_vec();

resize_or_reallocate_account(account, payer, system_program, new_size as usize)?;

sol_memcpy(
&mut account.data.borrow_mut()[(new_plugin_offset as usize)..],
&src,
src.len(),
);
// SAFETY: `borrow_mut` will always return a valid pointer.
// new_plugin_offset is derived from plugin_offset and size_diff using
// checked arithmetic, so it will always be less than or equal to account.data_len().
// This will fail and revert state if there is a memory violation.
unsafe {
let base = account.data.borrow_mut().as_mut_ptr();
sol_memmove(
base.add(new_plugin_offset as usize),
base.add(plugin_offset as usize),
registry_offset - plugin_offset as usize,
);
}

plugin_header.save(account, new_core_size as usize)?;

Expand Down
22 changes: 13 additions & 9 deletions programs/mpl-core/src/processor/update_external_plugin_adapter.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use borsh::{BorshDeserialize, BorshSerialize};
use mpl_utils::assert_signer;
use solana_program::{
account_info::AccountInfo, entrypoint::ProgramResult, msg, program_memory::sol_memcpy,
account_info::AccountInfo, entrypoint::ProgramResult, msg, program_memory::sol_memmove,
};

use crate::{
Expand Down Expand Up @@ -239,16 +239,20 @@ fn process_update_external_plugin_adapter<'a, T: DataBlob + SolanaAccount>(
.checked_add(plugin_size_diff)
.ok_or(MplCoreError::NumericalOverflow)?;

// //TODO: This is memory intensive, we should use memmove instead probably.
let src = account.data.borrow()[(next_plugin_offset as usize)..registry_offset].to_vec();

resize_or_reallocate_account(account, payer, system_program, new_size as usize)?;

sol_memcpy(
&mut account.data.borrow_mut()[(new_next_plugin_offset as usize)..],
&src,
src.len(),
);
// SAFETY: `borrow_mut` will always return a valid pointer.
// new_next_plugin_offset is derived from next_plugin_offset and size_diff using
// checked arithmetic, so it will always be less than or equal to account.data_len().
// This will fail and revert state if there is a memory violation.
unsafe {
let base = account.data.borrow_mut().as_mut_ptr();
sol_memmove(
base.add(new_next_plugin_offset as usize),
base.add(next_plugin_offset as usize),
registry_offset - next_plugin_offset as usize,
);
}

plugin_header.save(account, core.get_size())?;

Expand Down
22 changes: 13 additions & 9 deletions programs/mpl-core/src/processor/update_plugin.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use borsh::{BorshDeserialize, BorshSerialize};
use mpl_utils::assert_signer;
use solana_program::{
account_info::AccountInfo, entrypoint::ProgramResult, msg, program_memory::sol_memcpy,
account_info::AccountInfo, entrypoint::ProgramResult, msg, program_memory::sol_memmove,
};

use crate::{
Expand Down Expand Up @@ -185,16 +185,20 @@ fn process_update_plugin<'a, T: DataBlob + SolanaAccount>(
.checked_add(size_diff)
.ok_or(MplCoreError::NumericalOverflow)?;

// //TODO: This is memory intensive, we should use memmove instead probably.
let src = account.data.borrow()[(next_plugin_offset as usize)..registry_offset].to_vec();

resize_or_reallocate_account(account, payer, system_program, new_size as usize)?;

sol_memcpy(
&mut account.data.borrow_mut()[(new_next_plugin_offset as usize)..],
&src,
src.len(),
);
// SAFETY: `borrow_mut` will always return a valid pointer.
// new_next_plugin_offset is derived from next_plugin_offset and size_diff using
// checked arithmetic, so it will always be less than or equal to account.data_len().
// This will fail and revert state if there is a memory violation.
unsafe {
let base = account.data.borrow_mut().as_mut_ptr();
sol_memmove(
base.add(new_next_plugin_offset as usize),
base.add(next_plugin_offset as usize),
registry_offset - (next_plugin_offset as usize),
);
}

plugin_header.save(account, core.get_size())?;

Expand Down
Loading