Skip to content

Commit

Permalink
Refactor: move instructions sysvar serialization out of Message (#22544)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jstarry
jstarry authored Jan 20, 2022
1 parent f8db314 commit 7ba57e7
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 271 deletions.
2 changes: 1 addition & 1 deletion runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl Accounts {
message: &SanitizedMessage,
is_owned_by_sysvar: bool,
) -> AccountSharedData {
let data = construct_instructions_data(message);
let data = construct_instructions_data(&message.decompile_instructions());
let owner = if is_owned_by_sysvar {
sysvar::id()
} else {
Expand Down
11 changes: 6 additions & 5 deletions sdk/benches/serialize_instructions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use {
instruction::{AccountMeta, Instruction},
message::{Message, SanitizedMessage},
pubkey::{self, Pubkey},
sysvar::instructions,
sysvar::instructions::{self, construct_instructions_data},
},
std::convert::TryFrom,
test::Bencher,
Expand All @@ -28,13 +28,14 @@ fn bench_bincode_instruction_serialize(b: &mut Bencher) {
}

#[bench]
fn bench_manual_instruction_serialize(b: &mut Bencher) {
fn bench_construct_instructions_data(b: &mut Bencher) {
let instructions = make_instructions();
let message =
SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique())))
.unwrap();
b.iter(|| {
test::black_box(message.serialize_instructions());
let instructions = message.decompile_instructions();
test::black_box(construct_instructions_data(&instructions));
});
}

Expand All @@ -53,7 +54,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) {
let message =
SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique())))
.unwrap();
let serialized = message.serialize_instructions();
let serialized = construct_instructions_data(&message.decompile_instructions());
b.iter(|| {
for i in 0..instructions.len() {
#[allow(deprecated)]
Expand All @@ -68,7 +69,7 @@ fn bench_manual_instruction_deserialize_single(b: &mut Bencher) {
let message =
SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique())))
.unwrap();
let serialized = message.serialize_instructions();
let serialized = construct_instructions_data(&message.decompile_instructions());
b.iter(|| {
#[allow(deprecated)]
test::black_box(instructions::load_instruction_at(3, &serialized).unwrap());
Expand Down
147 changes: 2 additions & 145 deletions sdk/program/src/message/legacy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ use {
message::MessageHeader,
pubkey::Pubkey,
sanitize::{Sanitize, SanitizeError},
serialize_utils::{
append_slice, append_u16, append_u8, read_pubkey, read_slice, read_u16, read_u8,
},
short_vec, system_instruction, system_program, sysvar, wasm_bindgen,
},
lazy_static::lazy_static,
Expand Down Expand Up @@ -400,100 +397,13 @@ impl Message {
(writable_keys, readonly_keys)
}

// First encode the number of instructions:
// [0..2 - num_instructions
//
// Then a table of offsets of where to find them in the data
// 3..2 * num_instructions table of instruction offsets
//
// Each instruction is then encoded as:
// 0..2 - num_accounts
// 2 - meta_byte -> (bit 0 signer, bit 1 is_writable)
// 3..35 - pubkey - 32 bytes
// 35..67 - program_id
// 67..69 - data len - u16
// 69..data_len - data
#[deprecated]
pub fn serialize_instructions(&self) -> Vec<u8> {
// 64 bytes is a reasonable guess, calculating exactly is slower in benchmarks
let mut data = Vec::with_capacity(self.instructions.len() * (32 * 2));
append_u16(&mut data, self.instructions.len() as u16);
for _ in 0..self.instructions.len() {
append_u16(&mut data, 0);
}
for (i, instruction) in self.instructions.iter().enumerate() {
let start_instruction_offset = data.len() as u16;
let start = 2 + (2 * i);
data[start..start + 2].copy_from_slice(&start_instruction_offset.to_le_bytes());
append_u16(&mut data, instruction.accounts.len() as u16);
for account_index in &instruction.accounts {
let account_index = *account_index as usize;
let is_signer = self.is_signer(account_index);
let is_writable = self.is_writable(account_index);
let mut meta_byte = 0;
if is_signer {
meta_byte |= 1 << Self::IS_SIGNER_BIT;
}
if is_writable {
meta_byte |= 1 << Self::IS_WRITABLE_BIT;
}
append_u8(&mut data, meta_byte);
append_slice(&mut data, self.account_keys[account_index].as_ref());
}

let program_id = &self.account_keys[instruction.program_id_index as usize];
append_slice(&mut data, program_id.as_ref());
append_u16(&mut data, instruction.data.len() as u16);
append_slice(&mut data, &instruction.data);
}
data
}

const IS_SIGNER_BIT: usize = 0;
const IS_WRITABLE_BIT: usize = 1;

pub fn deserialize_instruction(
index: usize,
data: &[u8],
) -> Result<Instruction, SanitizeError> {
let mut current = 0;
let num_instructions = read_u16(&mut current, data)?;
if index >= num_instructions as usize {
return Err(SanitizeError::IndexOutOfBounds);
}

// index into the instruction byte-offset table.
current += index * 2;
let start = read_u16(&mut current, data)?;

current = start as usize;
let num_accounts = read_u16(&mut current, data)?;
let mut accounts = Vec::with_capacity(num_accounts as usize);
for _ in 0..num_accounts {
let meta_byte = read_u8(&mut current, data)?;
let mut is_signer = false;
let mut is_writable = false;
if meta_byte & (1 << Self::IS_SIGNER_BIT) != 0 {
is_signer = true;
}
if meta_byte & (1 << Self::IS_WRITABLE_BIT) != 0 {
is_writable = true;
}
let pubkey = read_pubkey(&mut current, data)?;
accounts.push(AccountMeta {
pubkey,
is_signer,
is_writable,
});
}
let program_id = read_pubkey(&mut current, data)?;
let data_len = read_u16(&mut current, data)?;
let data = read_slice(&mut current, data, data_len as usize)?;
Ok(Instruction {
program_id,
accounts,
data,
})
#[allow(deprecated)]
sysvar::instructions::load_instruction_at(index, data)
}

pub fn signer_keys(&self) -> Vec<&Pubkey> {
Expand Down Expand Up @@ -930,59 +840,6 @@ mod tests {
);
}

#[test]
fn test_decompile_instructions() {
solana_logger::setup();
let program_id0 = Pubkey::new_unique();
let program_id1 = Pubkey::new_unique();
let id0 = Pubkey::new_unique();
let id1 = Pubkey::new_unique();
let id2 = Pubkey::new_unique();
let id3 = Pubkey::new_unique();
let instructions = vec![
Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id0, false)]),
Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id1, true)]),
Instruction::new_with_bincode(
program_id1,
&0,
vec![AccountMeta::new_readonly(id2, false)],
),
Instruction::new_with_bincode(
program_id1,
&0,
vec![AccountMeta::new_readonly(id3, true)],
),
];

let message = Message::new(&instructions, Some(&id1));
let serialized = message.serialize_instructions();
for (i, instruction) in instructions.iter().enumerate() {
assert_eq!(
Message::deserialize_instruction(i, &serialized).unwrap(),
*instruction
);
}
}

#[test]
fn test_decompile_instructions_out_of_bounds() {
solana_logger::setup();
let program_id0 = Pubkey::new_unique();
let id0 = Pubkey::new_unique();
let id1 = Pubkey::new_unique();
let instructions = vec![
Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id0, false)]),
Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id1, true)]),
];

let message = Message::new(&instructions, Some(&id1));
let serialized = message.serialize_instructions();
assert_eq!(
Message::deserialize_instruction(instructions.len(), &serialized).unwrap_err(),
SanitizeError::IndexOutOfBounds,
);
}

#[test]
fn test_program_ids() {
let key0 = Pubkey::new_unique();
Expand Down
129 changes: 27 additions & 102 deletions sdk/program/src/message/sanitized.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ use {
program_utils::limited_deserialize,
pubkey::Pubkey,
sanitize::{Sanitize, SanitizeError},
serialize_utils::{append_slice, append_u16, append_u8},
solana_program::{system_instruction::SystemInstruction, system_program},
sysvar::instructions::{BorrowedAccountMeta, BorrowedInstruction},
},
bitflags::bitflags,
std::convert::TryFrom,
thiserror::Error,
};
Expand Down Expand Up @@ -57,14 +56,6 @@ impl TryFrom<LegacyMessage> for SanitizedMessage {
}
}

bitflags! {
struct InstructionsSysvarAccountMeta: u8 {
const NONE = 0b00000000;
const IS_SIGNER = 0b00000001;
const IS_WRITABLE = 0b00000010;
}
}

impl SanitizedMessage {
/// Return true if this message contains duplicate account keys
pub fn has_duplicates(&self) -> bool {
Expand Down Expand Up @@ -199,57 +190,6 @@ impl SanitizedMessage {
index < usize::from(self.header().num_required_signatures)
}

// First encode the number of instructions:
// [0..2 - num_instructions
//
// Then a table of offsets of where to find them in the data
// 3..2 * num_instructions table of instruction offsets
//
// Each instruction is then encoded as:
// 0..2 - num_accounts
// 2 - meta_byte -> (bit 0 signer, bit 1 is_writable)
// 3..35 - pubkey - 32 bytes
// 35..67 - program_id
// 67..69 - data len - u16
// 69..data_len - data
#[allow(clippy::integer_arithmetic)]
pub fn serialize_instructions(&self) -> Vec<u8> {
// 64 bytes is a reasonable guess, calculating exactly is slower in benchmarks
let mut data = Vec::with_capacity(self.instructions().len() * (32 * 2));
append_u16(&mut data, self.instructions().len() as u16);
for _ in 0..self.instructions().len() {
append_u16(&mut data, 0);
}
for (i, (program_id, instruction)) in self.program_instructions_iter().enumerate() {
let start_instruction_offset = data.len() as u16;
let start = 2 + (2 * i);
data[start..start + 2].copy_from_slice(&start_instruction_offset.to_le_bytes());
append_u16(&mut data, instruction.accounts.len() as u16);
for account_index in &instruction.accounts {
let account_index = *account_index as usize;
let is_signer = self.is_signer(account_index);
let is_writable = self.is_writable(account_index);
let mut account_meta = InstructionsSysvarAccountMeta::NONE;
if is_signer {
account_meta |= InstructionsSysvarAccountMeta::IS_SIGNER;
}
if is_writable {
account_meta |= InstructionsSysvarAccountMeta::IS_WRITABLE;
}
append_u8(&mut data, account_meta.bits());
append_slice(
&mut data,
self.get_account_key(account_index).unwrap().as_ref(),
);
}

append_slice(&mut data, program_id.as_ref());
append_u16(&mut data, instruction.data.len() as u16);
append_slice(&mut data, &instruction.data);
}
data
}

/// Return the resolved addresses for this message if it has any.
fn loaded_lookup_table_addresses(&self) -> Option<&LoadedAddresses> {
match &self {
Expand Down Expand Up @@ -288,6 +228,32 @@ impl SanitizedMessage {
})
}

/// Decompile message instructions without cloning account keys
pub fn decompile_instructions(&self) -> Vec<BorrowedInstruction> {
self.program_instructions_iter()
.map(|(program_id, instruction)| {
let accounts = instruction
.accounts
.iter()
.map(|account_index| {
let account_index = *account_index as usize;
BorrowedAccountMeta {
is_signer: self.is_signer(account_index),
is_writable: self.is_writable(account_index),
pubkey: self.get_account_key(account_index).unwrap(),
}
})
.collect();

BorrowedInstruction {
accounts,
data: &instruction.data,
program_id,
}
})
.collect()
}

/// Inspect all message keys for the bpf upgradeable loader
pub fn is_upgradeable_loader_present(&self) -> bool {
match self {
Expand Down Expand Up @@ -414,47 +380,6 @@ mod tests {
assert_eq!(v0_message.num_readonly_accounts(), 3);
}

#[test]
#[allow(deprecated)]
fn test_serialize_instructions() {
let program_id0 = Pubkey::new_unique();
let program_id1 = Pubkey::new_unique();
let id0 = Pubkey::new_unique();
let id1 = Pubkey::new_unique();
let id2 = Pubkey::new_unique();
let id3 = Pubkey::new_unique();
let instructions = vec![
Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id0, false)]),
Instruction::new_with_bincode(program_id0, &0, vec![AccountMeta::new(id1, true)]),
Instruction::new_with_bincode(
program_id1,
&0,
vec![AccountMeta::new_readonly(id2, false)],
),
Instruction::new_with_bincode(
program_id1,
&0,
vec![AccountMeta::new_readonly(id3, true)],
),
];

let message = LegacyMessage::new(&instructions, Some(&id1));
let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap();
let serialized = sanitized_message.serialize_instructions();

// assert that SanitizedMessage::serialize_instructions has the same behavior as the
// deprecated Message::serialize_instructions method
assert_eq!(serialized, message.serialize_instructions());

// assert that Message::deserialize_instruction is compatible with SanitizedMessage::serialize_instructions
for (i, instruction) in instructions.iter().enumerate() {
assert_eq!(
LegacyMessage::deserialize_instruction(i, &serialized).unwrap(),
*instruction
);
}
}

#[test]
fn test_try_compile_instruction() {
let key0 = Pubkey::new_unique();
Expand Down
Loading

0 comments on commit 7ba57e7

Please sign in to comment.