Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

CPI Account Reuse #19762

Merged
Lichtso merged 8 commits into from
Sep 18, 2021
Merged

CPI Account Reuse #19762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5671590
Moves construction of Account struct out of translate_accounts() into…
Lichtso Sep 8, 2021
c2d1097
Cleanup invoke_context borrowing chaos part 1.
Lichtso Sep 17, 2021
e1c6911
Cleanup invoke_context borrowing chaos part 2.
Lichtso Sep 17, 2021
f07fff8
Removes two account copy steps from InstructionProcessor::native_invo…
Lichtso Sep 16, 2021
533e381
Moves gathering of keyed_accounts and caller_write_privileges into In…
Lichtso Sep 17, 2021
96fada1
Moves gathering of program_indices into InstructionProcessor::create_…
Lichtso Sep 16, 2021
7311988
Explicitly routes the serialized account lengths to enable sharing of…
Lichtso Sep 16, 2021
81b3ba9
Recycles existing account structs in CPI syscall.
Lichtso Sep 17, 2021
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
356 changes: 140 additions & 216 deletions program-runtime/src/instruction_processor.rs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion programs/bpf/benches/bpf_loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) {

// Serialize account data
let keyed_accounts = invoke_context.get_keyed_accounts().unwrap();
let mut serialized = serialize_parameters(
let (mut serialized, _account_lengths) = serialize_parameters(
&bpf_loader::id(),
&solana_sdk::pubkey::new_rand(),
keyed_accounts,
Expand Down
3 changes: 2 additions & 1 deletion programs/bpf/tests/programs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ fn run_program(
let mut data = vec![];
file.read_to_end(&mut data).unwrap();
let loader_id = bpf_loader::id();
let parameter_bytes = serialize_parameters(
let (parameter_bytes, account_lengths) = serialize_parameters(
&bpf_loader::id(),
program_id,
&parameter_accounts,
Expand Down Expand Up @@ -282,6 +282,7 @@ fn run_program(
&bpf_loader::id(),
parameter_accounts,
parameter_bytes.as_slice(),
&account_lengths,
)
.unwrap();
}
Expand Down
9 changes: 7 additions & 2 deletions programs/bpf_loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ impl Executor for BpfExecutor {

let mut serialize_time = Measure::start("serialize");
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let mut parameter_bytes =
let (mut parameter_bytes, account_lengths) =
serialize_parameters(loader_id, program_id, keyed_accounts, instruction_data)?;
serialize_time.stop();
let mut create_vm_time = Measure::start("create_vm");
Expand Down Expand Up @@ -955,7 +955,12 @@ impl Executor for BpfExecutor {
}
let mut deserialize_time = Measure::start("deserialize");
let keyed_accounts = invoke_context.get_keyed_accounts()?;
deserialize_parameters(loader_id, keyed_accounts, parameter_bytes.as_slice())?;
deserialize_parameters(
loader_id,
keyed_accounts,
parameter_bytes.as_slice(),
&account_lengths,
)?;
deserialize_time.stop();
invoke_context.update_timing(
serialize_time.as_us(),
Expand Down
51 changes: 37 additions & 14 deletions programs/bpf_loader/src/serialization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,31 @@ pub fn serialize_parameters(
program_id: &Pubkey,
keyed_accounts: &[KeyedAccount],
data: &[u8],
) -> Result<AlignedMemory, InstructionError> {
) -> Result<(AlignedMemory, Vec<usize>), InstructionError> {
if *loader_id == bpf_loader_deprecated::id() {
serialize_parameters_unaligned(program_id, keyed_accounts, data)
} else {
serialize_parameters_aligned(program_id, keyed_accounts, data)
}
.and_then(|buffer| {
let account_lengths = keyed_accounts
.iter()
.map(|keyed_account| keyed_account.data_len())
.collect::<Result<Vec<usize>, InstructionError>>()?;
Ok((buffer, account_lengths))
})
}

pub fn deserialize_parameters(
loader_id: &Pubkey,
keyed_accounts: &[KeyedAccount],
buffer: &[u8],
account_lengths: &[usize],
) -> Result<(), InstructionError> {
if *loader_id == bpf_loader_deprecated::id() {
deserialize_parameters_unaligned(keyed_accounts, buffer)
deserialize_parameters_unaligned(keyed_accounts, buffer, account_lengths)
} else {
deserialize_parameters_aligned(keyed_accounts, buffer)
deserialize_parameters_aligned(keyed_accounts, buffer, account_lengths)
}
}

Expand Down Expand Up @@ -126,9 +134,14 @@ pub fn serialize_parameters_unaligned(
pub fn deserialize_parameters_unaligned(
keyed_accounts: &[KeyedAccount],
buffer: &[u8],
account_lengths: &[usize],
) -> Result<(), InstructionError> {
let mut start = size_of::<u64>(); // number of accounts
for (i, keyed_account) in keyed_accounts.iter().enumerate() {
for (i, (keyed_account, _pre_len)) in keyed_accounts
.iter()
.zip(account_lengths.iter())
.enumerate()
{
let (is_dup, _) = is_dup(&keyed_accounts[..i], keyed_account);
start += 1; // is_dup
if !is_dup {
Expand Down Expand Up @@ -247,9 +260,14 @@ pub fn serialize_parameters_aligned(
pub fn deserialize_parameters_aligned(
keyed_accounts: &[KeyedAccount],
buffer: &[u8],
account_lengths: &[usize],
Copy link
Copy Markdown
Contributor

@jackcmay jackcmay Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is neccessary because a CPI call will change the length of the account within this call?

Copy link
Copy Markdown
Contributor Author

@Lichtso Lichtso Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was the missing puzzle piece I searched for the entire day.
The hidden purpose of the additional temporary account structs.

Because once the CPI calls recycle the existing account structs, it overwrites the length and so the deserialize of the caller program afterwards parses and writes back garbage.

) -> Result<(), InstructionError> {
let mut start = size_of::<u64>(); // number of accounts
for (i, keyed_account) in keyed_accounts.iter().enumerate() {
for (i, (keyed_account, pre_len)) in keyed_accounts
.iter()
.zip(account_lengths.iter())
.enumerate()
{
let (is_dup, _) = is_dup(&keyed_accounts[..i], keyed_account);
start += size_of::<u8>(); // position
if is_dup {
Expand All @@ -265,18 +283,17 @@ pub fn deserialize_parameters_aligned(
start += size_of::<Pubkey>(); // owner
account.set_lamports(LittleEndian::read_u64(&buffer[start..]));
start += size_of::<u64>(); // lamports
let pre_len = account.data().len();
let post_len = LittleEndian::read_u64(&buffer[start..]) as usize;
start += size_of::<u64>(); // data length
let mut data_end = start + pre_len;
if post_len != pre_len
&& (post_len.saturating_sub(pre_len)) <= MAX_PERMITTED_DATA_INCREASE
let mut data_end = start + *pre_len;
if post_len != *pre_len
&& (post_len.saturating_sub(*pre_len)) <= MAX_PERMITTED_DATA_INCREASE
{
data_end = start + post_len;
}

account.set_data_from_slice(&buffer[start..data_end]);
start += pre_len + MAX_PERMITTED_DATA_INCREASE; // data
start += *pre_len + MAX_PERMITTED_DATA_INCREASE; // data
start += (start as *const u8).align_offset(align_of::<u128>());
start += size_of::<u64>(); // rent_epoch
}
Expand Down Expand Up @@ -392,7 +409,7 @@ mod tests {

// check serialize_parameters_aligned

let mut serialized = serialize_parameters(
let (mut serialized, account_lengths) = serialize_parameters(
&bpf_loader::id(),
&program_id,
&keyed_accounts,
Expand Down Expand Up @@ -445,8 +462,13 @@ mod tests {
}
})
.collect();
deserialize_parameters(&bpf_loader::id(), &de_keyed_accounts, serialized.as_slice())
.unwrap();
deserialize_parameters(
&bpf_loader::id(),
&de_keyed_accounts,
serialized.as_slice(),
&account_lengths,
)
.unwrap();
for ((account, de_keyed_account), key) in
accounts.iter().zip(de_keyed_accounts).zip(keys.clone())
{
Expand All @@ -458,7 +480,7 @@ mod tests {

// check serialize_parameters_unaligned

let mut serialized = serialize_parameters(
let (mut serialized, account_lengths) = serialize_parameters(
&bpf_loader_deprecated::id(),
&program_id,
&keyed_accounts,
Expand Down Expand Up @@ -497,6 +519,7 @@ mod tests {
&bpf_loader_deprecated::id(),
&de_keyed_accounts,
serialized.as_slice(),
&account_lengths,
)
.unwrap();
for ((account, de_keyed_account), key) in
Expand Down
Loading