From 93b75bf136ee802fc867c4a6d215cf8a1ed8a147 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Wed, 3 Jan 2024 18:05:28 +0900 Subject: [PATCH 1/6] add `Pod` and `Zeroable` for `RecordData` --- Cargo.lock | 2 + record/program/Cargo.toml | 2 + record/program/src/instruction.rs | 131 +++++++++++++++++++----------- record/program/src/processor.rs | 35 ++++---- record/program/src/state.rs | 41 ++++++++-- 5 files changed, 137 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 606b17fb932..e351d490d9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7112,11 +7112,13 @@ name = "spl-record" version = "0.1.0" dependencies = [ "borsh 0.10.3", + "bytemuck", "num-derive 0.4.1", "num-traits", "solana-program", "solana-program-test", "solana-sdk", + "spl-pod 0.1.0", "thiserror", ] diff --git a/record/program/Cargo.toml b/record/program/Cargo.toml index 4b8c6a0800c..02d1fbd8aa3 100644 --- a/record/program/Cargo.toml +++ b/record/program/Cargo.toml @@ -13,10 +13,12 @@ test-sbf = [] [dependencies] borsh = "0.10" +bytemuck = { version = "1.14.0", features = ["derive"] } num-derive = "0.4" num-traits = "0.2" solana-program = "1.17.6" thiserror = "1.0" +spl-pod = { version = "0.1", path = "../../libraries/pod" } [dev-dependencies] solana-program-test = "1.17.6" diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index 045af3184b0..4f5585e5452 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -5,8 +5,10 @@ use { borsh::{BorshDeserialize, BorshSerialize}, solana_program::{ instruction::{AccountMeta, Instruction}, + program_error::ProgramError, pubkey::Pubkey, }, + std::mem::size_of, }; /// Instructions supported by the program @@ -53,28 +55,70 @@ pub enum RecordInstruction { CloseAccount, } +impl RecordInstruction { + /// Unpacks a byte buffer into a [RecordInstruction]. + pub fn unpack(input: &[u8]) -> Result { + let (&tag, rest) = input + .split_first() + .ok_or(ProgramError::InvalidInstructionData)?; + Ok(match tag { + 0 => Self::Initialize, + 1 => { + const U64_BYTES: usize = 8; + let offset = rest + .get(..U64_BYTES) + .and_then(|slice| slice.try_into().ok()) + .map(u64::from_le_bytes) + .ok_or(ProgramError::InvalidInstructionData)?; + let data = rest[U64_BYTES..].to_vec(); + + Self::Write { offset, data } + } + 2 => Self::SetAuthority, + 3 => Self::CloseAccount, + _ => return Err(ProgramError::InvalidInstructionData), + }) + } + + /// Packs a [RecordInstruction] into a byte buffer. + pub fn pack(&self) -> Vec { + let mut buf = Vec::with_capacity(size_of::()); + match self { + Self::Initialize => buf.push(0), + Self::Write { offset, data } => { + buf.push(1); + buf.extend_from_slice(&offset.to_le_bytes()); + buf.extend_from_slice(data); + } + Self::SetAuthority => buf.push(2), + Self::CloseAccount => buf.push(3), + }; + buf + } +} + /// Create a `RecordInstruction::Initialize` instruction pub fn initialize(record_account: &Pubkey, authority: &Pubkey) -> Instruction { - Instruction::new_with_borsh( - id(), - &RecordInstruction::Initialize, - vec![ + Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(*record_account, false), AccountMeta::new_readonly(*authority, false), ], - ) + data: RecordInstruction::Initialize.pack(), + } } /// Create a `RecordInstruction::Write` instruction pub fn write(record_account: &Pubkey, signer: &Pubkey, offset: u64, data: Vec) -> Instruction { - Instruction::new_with_borsh( - id(), - &RecordInstruction::Write { offset, data }, - vec![ + Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(*record_account, false), AccountMeta::new_readonly(*signer, true), ], - ) + data: RecordInstruction::Write { offset, data }.pack(), + } } /// Create a `RecordInstruction::SetAuthority` instruction @@ -83,92 +127,81 @@ pub fn set_authority( signer: &Pubkey, new_authority: &Pubkey, ) -> Instruction { - Instruction::new_with_borsh( - id(), - &RecordInstruction::SetAuthority, - vec![ + Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(*record_account, false), AccountMeta::new_readonly(*signer, true), AccountMeta::new_readonly(*new_authority, false), ], - ) + data: RecordInstruction::SetAuthority.pack(), + } } /// Create a `RecordInstruction::CloseAccount` instruction pub fn close_account(record_account: &Pubkey, signer: &Pubkey, receiver: &Pubkey) -> Instruction { - Instruction::new_with_borsh( - id(), - &RecordInstruction::CloseAccount, - vec![ + Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(*record_account, false), AccountMeta::new_readonly(*signer, true), AccountMeta::new(*receiver, false), ], - ) + data: RecordInstruction::CloseAccount.pack(), + } } #[cfg(test)] mod tests { - use {super::*, crate::state::tests::TEST_DATA, solana_program::program_error::ProgramError}; + use { + super::*, crate::state::tests::TEST_DATA, solana_program::program_error::ProgramError, + spl_pod::bytemuck::pod_bytes_of, + }; #[test] fn serialize_initialize() { let instruction = RecordInstruction::Initialize; let expected = vec![0]; - assert_eq!(instruction.try_to_vec().unwrap(), expected); - assert_eq!( - RecordInstruction::try_from_slice(&expected).unwrap(), - instruction - ); + assert_eq!(instruction.pack(), expected); + assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); } #[test] fn serialize_write() { - let data = TEST_DATA.try_to_vec().unwrap(); + let data = pod_bytes_of(&TEST_DATA); let offset = 0u64; let instruction = RecordInstruction::Write { offset: 0, - data: data.clone(), + data: data.to_vec(), }; let mut expected = vec![1]; expected.extend_from_slice(&offset.to_le_bytes()); - expected.append(&mut data.try_to_vec().unwrap()); - assert_eq!(instruction.try_to_vec().unwrap(), expected); - assert_eq!( - RecordInstruction::try_from_slice(&expected).unwrap(), - instruction - ); + expected.extend_from_slice(data); + assert_eq!(instruction.pack(), expected); + assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); } #[test] fn serialize_set_authority() { let instruction = RecordInstruction::SetAuthority; let expected = vec![2]; - assert_eq!(instruction.try_to_vec().unwrap(), expected); - assert_eq!( - RecordInstruction::try_from_slice(&expected).unwrap(), - instruction - ); + assert_eq!(instruction.pack(), expected); + assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); } #[test] fn serialize_close_account() { let instruction = RecordInstruction::CloseAccount; let expected = vec![3]; - assert_eq!(instruction.try_to_vec().unwrap(), expected); - assert_eq!( - RecordInstruction::try_from_slice(&expected).unwrap(), - instruction - ); + assert_eq!(instruction.pack(), expected); + assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); } #[test] fn deserialize_invalid_instruction() { let mut expected = vec![12]; - expected.append(&mut TEST_DATA.try_to_vec().unwrap()); - let err: ProgramError = RecordInstruction::try_from_slice(&expected) - .unwrap_err() - .into(); - assert!(matches!(err, ProgramError::BorshIoError(_))); + expected.append(&mut pod_bytes_of(&TEST_DATA).to_vec()); + let err: ProgramError = RecordInstruction::unpack(&expected).unwrap_err(); + assert_eq!(err, ProgramError::InvalidInstructionData); } } diff --git a/record/program/src/processor.rs b/record/program/src/processor.rs index 451e2312b67..bcff815de96 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -6,7 +6,6 @@ use { instruction::RecordInstruction, state::{Data, RecordData}, }, - borsh::BorshDeserialize, solana_program::{ account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, @@ -15,6 +14,7 @@ use { program_pack::IsInitialized, pubkey::Pubkey, }, + spl_pod::bytemuck::{pod_from_bytes, pod_from_bytes_mut}, }; fn check_authority(authority_info: &AccountInfo, expected_authority: &Pubkey) -> ProgramResult { @@ -35,7 +35,7 @@ pub fn process_instruction( accounts: &[AccountInfo], input: &[u8], ) -> ProgramResult { - let instruction = RecordInstruction::try_from_slice(input)?; + let instruction = RecordInstruction::unpack(input)?; let account_info_iter = &mut accounts.iter(); match instruction { @@ -45,7 +45,8 @@ pub fn process_instruction( let data_info = next_account_info(account_info_iter)?; let authority_info = next_account_info(account_info_iter)?; - let mut account_data = RecordData::try_from_slice(*data_info.data.borrow())?; + let raw_data = &mut data_info.data.borrow_mut(); + let account_data = pod_from_bytes_mut::(raw_data)?; if account_data.is_initialized() { msg!("Record account already initialized"); return Err(ProgramError::AccountAlreadyInitialized); @@ -53,20 +54,22 @@ pub fn process_instruction( account_data.authority = *authority_info.key; account_data.version = RecordData::CURRENT_VERSION; - borsh::to_writer(&mut data_info.data.borrow_mut()[..], &account_data) - .map_err(|e| e.into()) + Ok(()) } RecordInstruction::Write { offset, data } => { msg!("RecordInstruction::Write"); let data_info = next_account_info(account_info_iter)?; let authority_info = next_account_info(account_info_iter)?; - let account_data = RecordData::try_from_slice(&data_info.data.borrow())?; - if !account_data.is_initialized() { - msg!("Record account not initialized"); - return Err(ProgramError::UninitializedAccount); + { + let raw_data = &data_info.data.borrow(); + let account_data = pod_from_bytes::(raw_data)?; + if !account_data.is_initialized() { + msg!("Record account not initialized"); + return Err(ProgramError::UninitializedAccount); + } + check_authority(authority_info, &account_data.authority)?; } - check_authority(authority_info, &account_data.authority)?; let start = RecordData::WRITABLE_START_INDEX.saturating_add(offset as usize); let end = start.saturating_add(data.len()); if end > data_info.data.borrow().len() { @@ -82,15 +85,15 @@ pub fn process_instruction( let data_info = next_account_info(account_info_iter)?; let authority_info = next_account_info(account_info_iter)?; let new_authority_info = next_account_info(account_info_iter)?; - let mut account_data = RecordData::try_from_slice(&data_info.data.borrow())?; + let raw_data = &mut data_info.data.borrow_mut(); + let account_data = pod_from_bytes_mut::(raw_data)?; if !account_data.is_initialized() { msg!("Record account not initialized"); return Err(ProgramError::UninitializedAccount); } check_authority(authority_info, &account_data.authority)?; account_data.authority = *new_authority_info.key; - borsh::to_writer(&mut data_info.data.borrow_mut()[..], &account_data) - .map_err(|e| e.into()) + Ok(()) } RecordInstruction::CloseAccount => { @@ -98,7 +101,8 @@ pub fn process_instruction( let data_info = next_account_info(account_info_iter)?; let authority_info = next_account_info(account_info_iter)?; let destination_info = next_account_info(account_info_iter)?; - let mut account_data = RecordData::try_from_slice(&data_info.data.borrow())?; + let raw_data = &mut data_info.data.borrow_mut(); + let account_data = pod_from_bytes_mut::(raw_data)?; if !account_data.is_initialized() { msg!("Record not initialized"); return Err(ProgramError::UninitializedAccount); @@ -111,8 +115,7 @@ pub fn process_instruction( .checked_add(data_lamports) .ok_or(RecordError::Overflow)?; account_data.data = Data::default(); - borsh::to_writer(&mut data_info.data.borrow_mut()[..], &account_data) - .map_err(|e| e.into()) + Ok(()) } } } diff --git a/record/program/src/state.rs b/record/program/src/state.rs index b88b698255f..ca12d6593c6 100644 --- a/record/program/src/state.rs +++ b/record/program/src/state.rs @@ -1,11 +1,15 @@ //! Program state use { borsh::{BorshDeserialize, BorshSchema, BorshSerialize}, + bytemuck::{Pod, Zeroable}, solana_program::{program_pack::IsInitialized, pubkey::Pubkey}, }; /// Struct wrapping data and providing metadata -#[derive(Clone, Debug, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq)] +#[repr(C)] +#[derive( + Clone, Copy, Debug, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq, Pod, Zeroable, +)] pub struct RecordData { /// Struct version, allows for upgrades to the program pub version: u8, @@ -17,11 +21,26 @@ pub struct RecordData { pub data: Data, } +/// The length of the data contained in the account for testing. +const DATA_SIZE: usize = 8; + /// Struct just for data -#[derive(Clone, Debug, Default, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq)] +#[repr(C)] +#[derive( + Clone, + Copy, + Debug, + Default, + BorshSerialize, + BorshDeserialize, + BorshSchema, + PartialEq, + Pod, + Zeroable, +)] pub struct Data { /// The data contained by the account, could be anything or serializable - pub bytes: [u8; Self::DATA_SIZE], + pub bytes: [u8; DATA_SIZE], } impl Data { @@ -46,7 +65,11 @@ impl IsInitialized for RecordData { #[cfg(test)] pub mod tests { - use {super::*, solana_program::program_error::ProgramError}; + use { + super::*, + solana_program::program_error::ProgramError, + spl_pod::bytemuck::{pod_bytes_of, pod_from_bytes}, + }; /// Version for tests pub const TEST_VERSION: u8 = 1; @@ -68,10 +91,10 @@ pub mod tests { let mut expected = vec![TEST_VERSION]; expected.extend_from_slice(&TEST_PUBKEY.to_bytes()); expected.extend_from_slice(&TEST_DATA.bytes); - assert_eq!(TEST_RECORD_DATA.try_to_vec().unwrap(), expected); + assert_eq!(pod_bytes_of(&TEST_RECORD_DATA), expected); assert_eq!( - RecordData::try_from_slice(&expected).unwrap(), - TEST_RECORD_DATA + *pod_from_bytes::(&expected).unwrap(), + TEST_RECORD_DATA, ); } @@ -81,7 +104,7 @@ pub mod tests { let mut expected = vec![TEST_VERSION]; expected.extend_from_slice(&TEST_PUBKEY.to_bytes()); expected.extend_from_slice(&data); - let err: ProgramError = RecordData::try_from_slice(&expected).unwrap_err().into(); - assert!(matches!(err, ProgramError::BorshIoError(_))); + let err: ProgramError = pod_from_bytes::(&expected).unwrap_err(); + assert_eq!(err, ProgramError::InvalidArgument); } } From c82c1b7b8bd74452e2c0d616a89dd7b9f2009e19 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Wed, 3 Jan 2024 18:07:17 +0900 Subject: [PATCH 2/6] remove borsh dependency --- Cargo.lock | 1 - record/program/Cargo.toml | 1 - record/program/src/instruction.rs | 3 +-- record/program/src/state.rs | 18 ++---------------- 4 files changed, 3 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e351d490d9b..574821404b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7111,7 +7111,6 @@ dependencies = [ name = "spl-record" version = "0.1.0" dependencies = [ - "borsh 0.10.3", "bytemuck", "num-derive 0.4.1", "num-traits", diff --git a/record/program/Cargo.toml b/record/program/Cargo.toml index 02d1fbd8aa3..8cfb90a44ee 100644 --- a/record/program/Cargo.toml +++ b/record/program/Cargo.toml @@ -12,7 +12,6 @@ no-entrypoint = [] test-sbf = [] [dependencies] -borsh = "0.10" bytemuck = { version = "1.14.0", features = ["derive"] } num-derive = "0.4" num-traits = "0.2" diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index 4f5585e5452..e98995c683d 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -2,7 +2,6 @@ use { crate::id, - borsh::{BorshDeserialize, BorshSerialize}, solana_program::{ instruction::{AccountMeta, Instruction}, program_error::ProgramError, @@ -12,7 +11,7 @@ use { }; /// Instructions supported by the program -#[derive(Clone, Debug, BorshSerialize, BorshDeserialize, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum RecordInstruction { /// Create a new record /// diff --git a/record/program/src/state.rs b/record/program/src/state.rs index ca12d6593c6..2dfec2e0bf7 100644 --- a/record/program/src/state.rs +++ b/record/program/src/state.rs @@ -1,15 +1,12 @@ //! Program state use { - borsh::{BorshDeserialize, BorshSchema, BorshSerialize}, bytemuck::{Pod, Zeroable}, solana_program::{program_pack::IsInitialized, pubkey::Pubkey}, }; /// Struct wrapping data and providing metadata #[repr(C)] -#[derive( - Clone, Copy, Debug, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq, Pod, Zeroable, -)] +#[derive(Clone, Copy, Debug, PartialEq, Pod, Zeroable)] pub struct RecordData { /// Struct version, allows for upgrades to the program pub version: u8, @@ -26,18 +23,7 @@ const DATA_SIZE: usize = 8; /// Struct just for data #[repr(C)] -#[derive( - Clone, - Copy, - Debug, - Default, - BorshSerialize, - BorshDeserialize, - BorshSchema, - PartialEq, - Pod, - Zeroable, -)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Pod, Zeroable)] pub struct Data { /// The data contained by the account, could be anything or serializable pub bytes: [u8; DATA_SIZE], From 8e1ad6bd29e7d7a53b82843a88a988a45343bcc4 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Wed, 3 Jan 2024 18:33:27 +0900 Subject: [PATCH 3/6] remove borsh from tests --- record/program/tests/functional.rs | 119 ++++++++++++++++------------- 1 file changed, 67 insertions(+), 52 deletions(-) diff --git a/record/program/tests/functional.rs b/record/program/tests/functional.rs index 3582d13cbf8..05ab9bc6f78 100644 --- a/record/program/tests/functional.rs +++ b/record/program/tests/functional.rs @@ -1,9 +1,7 @@ #![cfg(feature = "test-sbf")] use { - borsh::BorshSerialize, solana_program::{ - borsh0_10::get_packed_len, instruction::{AccountMeta, Instruction, InstructionError}, pubkey::Pubkey, rent::Rent, @@ -14,6 +12,7 @@ use { signature::{Keypair, Signer}, transaction::{Transaction, TransactionError}, }, + spl_pod::bytemuck::{pod_bytes_of, pod_from_bytes, pod_get_packed_len}, spl_record::{ error::RecordError, id, instruction, @@ -37,8 +36,8 @@ async fn initialize_storage_account( system_instruction::create_account( &context.payer.pubkey(), &account.pubkey(), - 1.max(Rent::default().minimum_balance(get_packed_len::())), - get_packed_len::() as u64, + 1.max(Rent::default().minimum_balance(pod_get_packed_len::())), + pod_get_packed_len::() as u64, &id(), ), instruction::initialize(&account.pubkey(), &authority.pubkey()), @@ -46,7 +45,7 @@ async fn initialize_storage_account( &account.pubkey(), &authority.pubkey(), 0, - data.try_to_vec().unwrap(), + pod_bytes_of(&data).to_vec(), ), ], Some(&context.payer.pubkey()), @@ -69,12 +68,15 @@ async fn initialize_success() { let data = Data { bytes: [111u8; Data::DATA_SIZE], }; - initialize_storage_account(&mut context, &authority, &account, data.clone()).await; - let account_data = context + initialize_storage_account(&mut context, &authority, &account, data).await; + + let account = context .banks_client - .get_account_data_with_borsh::(account.pubkey()) + .get_account(account.pubkey()) .await + .unwrap() .unwrap(); + let account_data = pod_from_bytes::(&account.data).unwrap(); assert_eq!(account_data.data, data); assert_eq!(account_data.authority, authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); @@ -97,12 +99,17 @@ async fn initialize_with_seed_success() { &account, &authority.pubkey(), seed, - 1.max(Rent::default().minimum_balance(get_packed_len::())), - get_packed_len::() as u64, + 1.max(Rent::default().minimum_balance(pod_get_packed_len::())), + pod_get_packed_len::() as u64, &id(), ), instruction::initialize(&account, &authority.pubkey()), - instruction::write(&account, &authority.pubkey(), 0, data.try_to_vec().unwrap()), + instruction::write( + &account, + &authority.pubkey(), + 0, + pod_bytes_of(&data).to_vec(), + ), ], Some(&context.payer.pubkey()), &[&context.payer, &authority], @@ -113,11 +120,13 @@ async fn initialize_with_seed_success() { .process_transaction(transaction) .await .unwrap(); - let account_data = context + let account = context .banks_client - .get_account_data_with_borsh::(account) + .get_account(account) .await + .unwrap() .unwrap(); + let account_data = pod_from_bytes::(&account.data).unwrap(); assert_eq!(account_data.data, data); assert_eq!(account_data.authority, authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); @@ -172,7 +181,7 @@ async fn write_success() { &account.pubkey(), &authority.pubkey(), 0, - new_data.try_to_vec().unwrap(), + pod_bytes_of(&new_data).to_vec(), )], Some(&context.payer.pubkey()), &[&context.payer, &authority], @@ -184,11 +193,13 @@ async fn write_success() { .await .unwrap(); - let account_data = context + let account = context .banks_client - .get_account_data_with_borsh::(account.pubkey()) + .get_account(account.pubkey()) .await + .unwrap() .unwrap(); + let account_data = pod_from_bytes::(&account.data).unwrap(); assert_eq!(account_data.data, new_data); assert_eq!(account_data.authority, authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); @@ -214,7 +225,7 @@ async fn write_fail_wrong_authority() { &account.pubkey(), &wrong_authority.pubkey(), 0, - new_data.try_to_vec().unwrap(), + pod_bytes_of(&new_data).to_vec(), )], Some(&context.payer.pubkey()), &[&context.payer, &wrong_authority], @@ -245,20 +256,20 @@ async fn write_fail_unsigned() { }; initialize_storage_account(&mut context, &authority, &account, data).await; - let data = Data { + let data = pod_bytes_of(&Data { bytes: [200u8; Data::DATA_SIZE], - } - .try_to_vec() - .unwrap(); + }) + .to_vec(); + let transaction = Transaction::new_signed_with_payer( - &[Instruction::new_with_borsh( - id(), - &instruction::RecordInstruction::Write { offset: 0, data }, - vec![ + &[Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(account.pubkey(), false), AccountMeta::new_readonly(authority.pubkey(), false), ], - )], + data: instruction::RecordInstruction::Write { offset: 0, data }.pack(), + }], Some(&context.payer.pubkey()), &[&context.payer], context.last_blockhash, @@ -310,7 +321,7 @@ async fn close_account_success() { .unwrap(); assert_eq!( account.lamports, - 1.max(Rent::default().minimum_balance(get_packed_len::())) + 1.max(Rent::default().minimum_balance(pod_get_packed_len::())) ); } @@ -327,15 +338,15 @@ async fn close_account_fail_wrong_authority() { let wrong_authority = Keypair::new(); let transaction = Transaction::new_signed_with_payer( - &[Instruction::new_with_borsh( - id(), - &instruction::RecordInstruction::CloseAccount, - vec![ + &[Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(account.pubkey(), false), AccountMeta::new_readonly(wrong_authority.pubkey(), true), AccountMeta::new(Pubkey::new_unique(), false), ], - )], + data: instruction::RecordInstruction::CloseAccount.pack(), + }], Some(&context.payer.pubkey()), &[&context.payer, &wrong_authority], context.last_blockhash, @@ -366,15 +377,15 @@ async fn close_account_fail_unsigned() { initialize_storage_account(&mut context, &authority, &account, data).await; let transaction = Transaction::new_signed_with_payer( - &[Instruction::new_with_borsh( - id(), - &instruction::RecordInstruction::CloseAccount, - vec![ + &[Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(account.pubkey(), false), AccountMeta::new_readonly(authority.pubkey(), false), AccountMeta::new(Pubkey::new_unique(), false), ], - )], + data: instruction::RecordInstruction::CloseAccount.pack(), + }], Some(&context.payer.pubkey()), &[&context.payer], context.last_blockhash, @@ -418,11 +429,13 @@ async fn set_authority_success() { .await .unwrap(); - let account_data = context + let account_handle = context .banks_client - .get_account_data_with_borsh::(account.pubkey()) + .get_account(account.pubkey()) .await + .unwrap() .unwrap(); + let account_data = pod_from_bytes::(&account_handle.data).unwrap(); assert_eq!(account_data.authority, new_authority.pubkey()); let new_data = Data { @@ -433,7 +446,7 @@ async fn set_authority_success() { &account.pubkey(), &new_authority.pubkey(), 0, - new_data.try_to_vec().unwrap(), + pod_bytes_of(&new_data).to_vec(), )], Some(&context.payer.pubkey()), &[&context.payer, &new_authority], @@ -445,11 +458,13 @@ async fn set_authority_success() { .await .unwrap(); - let account_data = context + let account_handle = context .banks_client - .get_account_data_with_borsh::(account.pubkey()) + .get_account(account.pubkey()) .await + .unwrap() .unwrap(); + let account_data = pod_from_bytes::(&account_handle.data).unwrap(); assert_eq!(account_data.data, new_data); assert_eq!(account_data.authority, new_authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); @@ -468,15 +483,15 @@ async fn set_authority_fail_wrong_authority() { let wrong_authority = Keypair::new(); let transaction = Transaction::new_signed_with_payer( - &[Instruction::new_with_borsh( - id(), - &instruction::RecordInstruction::SetAuthority, - vec![ + &[Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(account.pubkey(), false), AccountMeta::new_readonly(wrong_authority.pubkey(), true), AccountMeta::new(Pubkey::new_unique(), false), ], - )], + data: instruction::RecordInstruction::SetAuthority.pack(), + }], Some(&context.payer.pubkey()), &[&context.payer, &wrong_authority], context.last_blockhash, @@ -507,15 +522,15 @@ async fn set_authority_fail_unsigned() { initialize_storage_account(&mut context, &authority, &account, data).await; let transaction = Transaction::new_signed_with_payer( - &[Instruction::new_with_borsh( - id(), - &instruction::RecordInstruction::SetAuthority, - vec![ + &[Instruction { + program_id: id(), + accounts: vec![ AccountMeta::new(account.pubkey(), false), AccountMeta::new_readonly(authority.pubkey(), false), AccountMeta::new(Pubkey::new_unique(), false), ], - )], + data: instruction::RecordInstruction::SetAuthority.pack(), + }], Some(&context.payer.pubkey()), &[&context.payer], context.last_blockhash, From 47569dacd6f954ee3c3af6cbca0d5f4ccf9f2c39 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Thu, 4 Jan 2024 10:05:25 +0900 Subject: [PATCH 4/6] remove unnecessary copy from vector --- record/program/src/instruction.rs | 21 ++++++++++----------- record/program/tests/functional.rs | 18 ++++++------------ 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index e98995c683d..cb5d5b5c6d8 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -12,7 +12,7 @@ use { /// Instructions supported by the program #[derive(Clone, Debug, PartialEq)] -pub enum RecordInstruction { +pub enum RecordInstruction<'a> { /// Create a new record /// /// Accounts expected by this instruction: @@ -31,7 +31,7 @@ pub enum RecordInstruction { /// Offset to start writing record, expressed as `u64`. offset: u64, /// Data to replace the existing record data - data: Vec, + data: &'a [u8], }, /// Update the authority of the provided record account @@ -54,9 +54,9 @@ pub enum RecordInstruction { CloseAccount, } -impl RecordInstruction { +impl<'a> RecordInstruction<'a> { /// Unpacks a byte buffer into a [RecordInstruction]. - pub fn unpack(input: &[u8]) -> Result { + pub fn unpack(input: &'a [u8]) -> Result { let (&tag, rest) = input .split_first() .ok_or(ProgramError::InvalidInstructionData)?; @@ -69,9 +69,11 @@ impl RecordInstruction { .and_then(|slice| slice.try_into().ok()) .map(u64::from_le_bytes) .ok_or(ProgramError::InvalidInstructionData)?; - let data = rest[U64_BYTES..].to_vec(); - Self::Write { offset, data } + Self::Write { + offset, + data: &rest[U64_BYTES..], + } } 2 => Self::SetAuthority, 3 => Self::CloseAccount, @@ -109,7 +111,7 @@ pub fn initialize(record_account: &Pubkey, authority: &Pubkey) -> Instruction { } /// Create a `RecordInstruction::Write` instruction -pub fn write(record_account: &Pubkey, signer: &Pubkey, offset: u64, data: Vec) -> Instruction { +pub fn write(record_account: &Pubkey, signer: &Pubkey, offset: u64, data: &[u8]) -> Instruction { Instruction { program_id: id(), accounts: vec![ @@ -169,10 +171,7 @@ mod tests { fn serialize_write() { let data = pod_bytes_of(&TEST_DATA); let offset = 0u64; - let instruction = RecordInstruction::Write { - offset: 0, - data: data.to_vec(), - }; + let instruction = RecordInstruction::Write { offset: 0, data }; let mut expected = vec![1]; expected.extend_from_slice(&offset.to_le_bytes()); expected.extend_from_slice(data); diff --git a/record/program/tests/functional.rs b/record/program/tests/functional.rs index 05ab9bc6f78..64e068f6604 100644 --- a/record/program/tests/functional.rs +++ b/record/program/tests/functional.rs @@ -45,7 +45,7 @@ async fn initialize_storage_account( &account.pubkey(), &authority.pubkey(), 0, - pod_bytes_of(&data).to_vec(), + pod_bytes_of(&data), ), ], Some(&context.payer.pubkey()), @@ -104,12 +104,7 @@ async fn initialize_with_seed_success() { &id(), ), instruction::initialize(&account, &authority.pubkey()), - instruction::write( - &account, - &authority.pubkey(), - 0, - pod_bytes_of(&data).to_vec(), - ), + instruction::write(&account, &authority.pubkey(), 0, pod_bytes_of(&data)), ], Some(&context.payer.pubkey()), &[&context.payer, &authority], @@ -181,7 +176,7 @@ async fn write_success() { &account.pubkey(), &authority.pubkey(), 0, - pod_bytes_of(&new_data).to_vec(), + pod_bytes_of(&new_data), )], Some(&context.payer.pubkey()), &[&context.payer, &authority], @@ -225,7 +220,7 @@ async fn write_fail_wrong_authority() { &account.pubkey(), &wrong_authority.pubkey(), 0, - pod_bytes_of(&new_data).to_vec(), + pod_bytes_of(&new_data), )], Some(&context.payer.pubkey()), &[&context.payer, &wrong_authority], @@ -258,8 +253,7 @@ async fn write_fail_unsigned() { let data = pod_bytes_of(&Data { bytes: [200u8; Data::DATA_SIZE], - }) - .to_vec(); + }); let transaction = Transaction::new_signed_with_payer( &[Instruction { @@ -446,7 +440,7 @@ async fn set_authority_success() { &account.pubkey(), &new_authority.pubkey(), 0, - pod_bytes_of(&new_data).to_vec(), + pod_bytes_of(&new_data), )], Some(&context.payer.pubkey()), &[&context.payer, &new_authority], From 0eac21ae07db37d5047325b9b64bf5676fc16c02 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Thu, 4 Jan 2024 10:51:19 +0900 Subject: [PATCH 5/6] add vector length to serialization --- record/program/src/instruction.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index cb5d5b5c6d8..6df52d3518b 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -63,16 +63,23 @@ impl<'a> RecordInstruction<'a> { Ok(match tag { 0 => Self::Initialize, 1 => { + const U32_BYTES: usize = 4; const U64_BYTES: usize = 8; let offset = rest .get(..U64_BYTES) .and_then(|slice| slice.try_into().ok()) .map(u64::from_le_bytes) .ok_or(ProgramError::InvalidInstructionData)?; + let (length, data) = rest[U64_BYTES..].split_at(U32_BYTES); + let length = u32::from_le_bytes( + length + .try_into() + .map_err(|_| ProgramError::InvalidInstructionData)?, + ) as usize; Self::Write { offset, - data: &rest[U64_BYTES..], + data: &data[..length], } } 2 => Self::SetAuthority, @@ -89,6 +96,7 @@ impl<'a> RecordInstruction<'a> { Self::Write { offset, data } => { buf.push(1); buf.extend_from_slice(&offset.to_le_bytes()); + buf.extend_from_slice(&(data.len() as u32).to_le_bytes()); buf.extend_from_slice(data); } Self::SetAuthority => buf.push(2), @@ -174,6 +182,7 @@ mod tests { let instruction = RecordInstruction::Write { offset: 0, data }; let mut expected = vec![1]; expected.extend_from_slice(&offset.to_le_bytes()); + expected.extend_from_slice(&(data.len() as u32).to_le_bytes()); expected.extend_from_slice(data); assert_eq!(instruction.pack(), expected); assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); From 37e89b694762c64dbb8a85de93e1a7411b9d6f1d Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Thu, 4 Jan 2024 11:03:27 +0900 Subject: [PATCH 6/6] clippy --- record/program/src/processor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/record/program/src/processor.rs b/record/program/src/processor.rs index bcff815de96..472740cb245 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -75,7 +75,7 @@ pub fn process_instruction( if end > data_info.data.borrow().len() { Err(ProgramError::AccountDataTooSmall) } else { - data_info.data.borrow_mut()[start..end].copy_from_slice(&data); + data_info.data.borrow_mut()[start..end].copy_from_slice(data); Ok(()) } }