From 8e4d5653211920a435087cc19d982f38b94485fb Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 09:05:39 +0900 Subject: [PATCH 1/3] remove `Data` type from `RecordData` --- record/program/src/instruction.rs | 9 +++----- record/program/src/processor.rs | 35 ++++++++++++++++++++++--------- record/program/src/state.rs | 28 ++----------------------- 3 files changed, 30 insertions(+), 42 deletions(-) diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index 6df52d3518b..0c2df24cde7 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -162,10 +162,7 @@ pub fn close_account(record_account: &Pubkey, signer: &Pubkey, receiver: &Pubkey #[cfg(test)] mod tests { - use { - super::*, crate::state::tests::TEST_DATA, solana_program::program_error::ProgramError, - spl_pod::bytemuck::pod_bytes_of, - }; + use {super::*, crate::state::tests::TEST_BYTES, solana_program::program_error::ProgramError}; #[test] fn serialize_initialize() { @@ -177,7 +174,7 @@ mod tests { #[test] fn serialize_write() { - let data = pod_bytes_of(&TEST_DATA); + let data = &TEST_BYTES; let offset = 0u64; let instruction = RecordInstruction::Write { offset: 0, data }; let mut expected = vec![1]; @@ -207,7 +204,7 @@ mod tests { #[test] fn deserialize_invalid_instruction() { let mut expected = vec![12]; - expected.append(&mut pod_bytes_of(&TEST_DATA).to_vec()); + expected.extend_from_slice(&TEST_BYTES); 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 472740cb245..8c6c5593984 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -1,11 +1,7 @@ //! Program state processor use { - crate::{ - error::RecordError, - instruction::RecordInstruction, - state::{Data, RecordData}, - }, + crate::{error::RecordError, instruction::RecordInstruction, state::RecordData}, solana_program::{ account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, @@ -46,7 +42,13 @@ pub fn process_instruction( let authority_info = next_account_info(account_info_iter)?; let raw_data = &mut data_info.data.borrow_mut(); - let account_data = pod_from_bytes_mut::(raw_data)?; + if raw_data.len() < RecordData::WRITABLE_START_INDEX { + return Err(ProgramError::InvalidAccountData); + } + + let account_data = pod_from_bytes_mut::( + &mut raw_data[..RecordData::WRITABLE_START_INDEX], + )?; if account_data.is_initialized() { msg!("Record account already initialized"); return Err(ProgramError::AccountAlreadyInitialized); @@ -63,7 +65,11 @@ pub fn process_instruction( let authority_info = next_account_info(account_info_iter)?; { let raw_data = &data_info.data.borrow(); - let account_data = pod_from_bytes::(raw_data)?; + if raw_data.len() < RecordData::WRITABLE_START_INDEX { + return Err(ProgramError::InvalidAccountData); + } + let account_data = + pod_from_bytes::(&raw_data[..RecordData::WRITABLE_START_INDEX])?; if !account_data.is_initialized() { msg!("Record account not initialized"); return Err(ProgramError::UninitializedAccount); @@ -86,7 +92,12 @@ pub fn process_instruction( let authority_info = next_account_info(account_info_iter)?; let new_authority_info = next_account_info(account_info_iter)?; let raw_data = &mut data_info.data.borrow_mut(); - let account_data = pod_from_bytes_mut::(raw_data)?; + if raw_data.len() < RecordData::WRITABLE_START_INDEX { + return Err(ProgramError::InvalidAccountData); + } + let account_data = pod_from_bytes_mut::( + &mut raw_data[..RecordData::WRITABLE_START_INDEX], + )?; if !account_data.is_initialized() { msg!("Record account not initialized"); return Err(ProgramError::UninitializedAccount); @@ -102,7 +113,12 @@ pub fn process_instruction( let authority_info = next_account_info(account_info_iter)?; let destination_info = next_account_info(account_info_iter)?; let raw_data = &mut data_info.data.borrow_mut(); - let account_data = pod_from_bytes_mut::(raw_data)?; + if raw_data.len() < RecordData::WRITABLE_START_INDEX { + return Err(ProgramError::InvalidAccountData); + } + let account_data = pod_from_bytes_mut::( + &mut raw_data[..RecordData::WRITABLE_START_INDEX], + )?; if !account_data.is_initialized() { msg!("Record not initialized"); return Err(ProgramError::UninitializedAccount); @@ -114,7 +130,6 @@ pub fn process_instruction( **destination_info.lamports.borrow_mut() = destination_starting_lamports .checked_add(data_lamports) .ok_or(RecordError::Overflow)?; - account_data.data = Data::default(); Ok(()) } } diff --git a/record/program/src/state.rs b/record/program/src/state.rs index 2dfec2e0bf7..32c8d3157ed 100644 --- a/record/program/src/state.rs +++ b/record/program/src/state.rs @@ -13,25 +13,6 @@ pub struct RecordData { /// The account allowed to update the data pub authority: Pubkey, - - /// The data contained by the account, could be anything serializable - pub data: Data, -} - -/// The length of the data contained in the account for testing. -const DATA_SIZE: usize = 8; - -/// Struct just for data -#[repr(C)] -#[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], -} - -impl Data { - /// very small data for easy testing - pub const DATA_SIZE: usize = 8; } impl RecordData { @@ -62,21 +43,17 @@ pub mod tests { /// Pubkey for tests pub const TEST_PUBKEY: Pubkey = Pubkey::new_from_array([100; 32]); /// Bytes for tests - pub const TEST_BYTES: [u8; Data::DATA_SIZE] = [42; Data::DATA_SIZE]; - /// Data for tests - pub const TEST_DATA: Data = Data { bytes: TEST_BYTES }; + pub const TEST_BYTES: [u8; 8] = [42; 8]; /// RecordData for tests pub const TEST_RECORD_DATA: RecordData = RecordData { version: TEST_VERSION, authority: TEST_PUBKEY, - data: TEST_DATA, }; #[test] fn serialize_data() { let mut expected = vec![TEST_VERSION]; expected.extend_from_slice(&TEST_PUBKEY.to_bytes()); - expected.extend_from_slice(&TEST_DATA.bytes); assert_eq!(pod_bytes_of(&TEST_RECORD_DATA), expected); assert_eq!( *pod_from_bytes::(&expected).unwrap(), @@ -86,10 +63,9 @@ pub mod tests { #[test] fn deserialize_invalid_slice() { - let data = [200; Data::DATA_SIZE - 1]; let mut expected = vec![TEST_VERSION]; expected.extend_from_slice(&TEST_PUBKEY.to_bytes()); - expected.extend_from_slice(&data); + expected.extend_from_slice(&TEST_BYTES); let err: ProgramError = pod_from_bytes::(&expected).unwrap_err(); assert_eq!(err, ProgramError::InvalidArgument); } From 944c8e1e9313af1144b140c9880363b2488d451c Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 09:06:13 +0900 Subject: [PATCH 2/3] remove `Data` from tests --- record/program/tests/functional.rs | 135 ++++++++++++----------------- 1 file changed, 57 insertions(+), 78 deletions(-) diff --git a/record/program/tests/functional.rs b/record/program/tests/functional.rs index 64e068f6604..6204c50731c 100644 --- a/record/program/tests/functional.rs +++ b/record/program/tests/functional.rs @@ -12,12 +12,9 @@ use { signature::{Keypair, Signer}, transaction::{Transaction, TransactionError}, }, - spl_pod::bytemuck::{pod_bytes_of, pod_from_bytes, pod_get_packed_len}, + spl_pod::bytemuck::{pod_from_bytes, pod_get_packed_len}, spl_record::{ - error::RecordError, - id, instruction, - processor::process_instruction, - state::{Data, RecordData}, + error::RecordError, id, instruction, processor::process_instruction, state::RecordData, }, }; @@ -29,24 +26,22 @@ async fn initialize_storage_account( context: &mut ProgramTestContext, authority: &Keypair, account: &Keypair, - data: Data, + data: &[u8], ) { + let account_length = pod_get_packed_len::() + .checked_add(data.len()) + .unwrap(); let transaction = Transaction::new_signed_with_payer( &[ system_instruction::create_account( &context.payer.pubkey(), &account.pubkey(), - 1.max(Rent::default().minimum_balance(pod_get_packed_len::())), - pod_get_packed_len::() as u64, + 1.max(Rent::default().minimum_balance(account_length)), + account_length as u64, &id(), ), instruction::initialize(&account.pubkey(), &authority.pubkey()), - instruction::write( - &account.pubkey(), - &authority.pubkey(), - 0, - pod_bytes_of(&data), - ), + instruction::write(&account.pubkey(), &authority.pubkey(), 0, data), ], Some(&context.payer.pubkey()), &[&context.payer, account, authority], @@ -65,9 +60,7 @@ async fn initialize_success() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [111u8; Data::DATA_SIZE], - }; + let data = &[111u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let account = context @@ -76,10 +69,11 @@ async fn initialize_success() { .await .unwrap() .unwrap(); - let account_data = pod_from_bytes::(&account.data).unwrap(); - assert_eq!(account_data.data, data); + let account_data = + pod_from_bytes::(&account.data[..RecordData::WRITABLE_START_INDEX]).unwrap(); assert_eq!(account_data.authority, authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); + assert_eq!(&account.data[RecordData::WRITABLE_START_INDEX..], data); } #[tokio::test] @@ -89,9 +83,10 @@ async fn initialize_with_seed_success() { let authority = Keypair::new(); let seed = "storage"; let account = Pubkey::create_with_seed(&authority.pubkey(), seed, &id()).unwrap(); - let data = Data { - bytes: [111u8; Data::DATA_SIZE], - }; + let data = &[111u8; 8]; + let account_length = pod_get_packed_len::() + .checked_add(data.len()) + .unwrap(); let transaction = Transaction::new_signed_with_payer( &[ system_instruction::create_account_with_seed( @@ -99,12 +94,12 @@ async fn initialize_with_seed_success() { &account, &authority.pubkey(), seed, - 1.max(Rent::default().minimum_balance(pod_get_packed_len::())), - pod_get_packed_len::() as u64, + 1.max(Rent::default().minimum_balance(account_length)), + account_length as u64, &id(), ), instruction::initialize(&account, &authority.pubkey()), - instruction::write(&account, &authority.pubkey(), 0, pod_bytes_of(&data)), + instruction::write(&account, &authority.pubkey(), 0, data), ], Some(&context.payer.pubkey()), &[&context.payer, &authority], @@ -121,10 +116,11 @@ async fn initialize_with_seed_success() { .await .unwrap() .unwrap(); - let account_data = pod_from_bytes::(&account.data).unwrap(); - assert_eq!(account_data.data, data); + let account_data = + pod_from_bytes::(&account.data[..RecordData::WRITABLE_START_INDEX]).unwrap(); assert_eq!(account_data.authority, authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); + assert_eq!(&account.data[RecordData::WRITABLE_START_INDEX..], data); } #[tokio::test] @@ -133,9 +129,7 @@ async fn initialize_twice_fail() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [111u8; Data::DATA_SIZE], - }; + let data = &[111u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let transaction = Transaction::new_signed_with_payer( &[instruction::initialize( @@ -163,20 +157,16 @@ async fn write_success() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; - let new_data = Data { - bytes: [200u8; Data::DATA_SIZE], - }; + let new_data = &[200u8; 8]; let transaction = Transaction::new_signed_with_payer( &[instruction::write( &account.pubkey(), &authority.pubkey(), 0, - pod_bytes_of(&new_data), + new_data, )], Some(&context.payer.pubkey()), &[&context.payer, &authority], @@ -194,10 +184,11 @@ async fn write_success() { .await .unwrap() .unwrap(); - let account_data = pod_from_bytes::(&account.data).unwrap(); - assert_eq!(account_data.data, new_data); + let account_data = + pod_from_bytes::(&account.data[..RecordData::WRITABLE_START_INDEX]).unwrap(); assert_eq!(account_data.authority, authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); + assert_eq!(&account.data[RecordData::WRITABLE_START_INDEX..], new_data); } #[tokio::test] @@ -206,21 +197,17 @@ async fn write_fail_wrong_authority() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; - let new_data = Data { - bytes: [200u8; Data::DATA_SIZE], - }; + let new_data = &[200u8; 8]; let wrong_authority = Keypair::new(); let transaction = Transaction::new_signed_with_payer( &[instruction::write( &account.pubkey(), &wrong_authority.pubkey(), 0, - pod_bytes_of(&new_data), + new_data, )], Some(&context.payer.pubkey()), &[&context.payer, &wrong_authority], @@ -246,14 +233,10 @@ async fn write_fail_unsigned() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; - let data = pod_bytes_of(&Data { - bytes: [200u8; Data::DATA_SIZE], - }); + let data = &[200u8; 8]; let transaction = Transaction::new_signed_with_payer( &[Instruction { @@ -285,9 +268,10 @@ async fn close_account_success() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; + let account_length = pod_get_packed_len::() + .checked_add(data.len()) + .unwrap(); initialize_storage_account(&mut context, &authority, &account, data).await; let recipient = Pubkey::new_unique(); @@ -315,7 +299,7 @@ async fn close_account_success() { .unwrap(); assert_eq!( account.lamports, - 1.max(Rent::default().minimum_balance(pod_get_packed_len::())) + 1.max(Rent::default().minimum_balance(account_length)) ); } @@ -325,9 +309,7 @@ async fn close_account_fail_wrong_authority() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let wrong_authority = Keypair::new(); @@ -365,9 +347,7 @@ async fn close_account_fail_unsigned() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8, 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let transaction = Transaction::new_signed_with_payer( @@ -401,9 +381,7 @@ async fn set_authority_success() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let new_authority = Keypair::new(); @@ -429,18 +407,18 @@ async fn set_authority_success() { .await .unwrap() .unwrap(); - let account_data = pod_from_bytes::(&account_handle.data).unwrap(); + let account_data = + pod_from_bytes::(&account_handle.data[..RecordData::WRITABLE_START_INDEX]) + .unwrap(); assert_eq!(account_data.authority, new_authority.pubkey()); - let new_data = Data { - bytes: [200u8; Data::DATA_SIZE], - }; + let new_data = &[200u8; 8]; let transaction = Transaction::new_signed_with_payer( &[instruction::write( &account.pubkey(), &new_authority.pubkey(), 0, - pod_bytes_of(&new_data), + new_data, )], Some(&context.payer.pubkey()), &[&context.payer, &new_authority], @@ -458,10 +436,15 @@ async fn set_authority_success() { .await .unwrap() .unwrap(); - let account_data = pod_from_bytes::(&account_handle.data).unwrap(); - assert_eq!(account_data.data, new_data); + let account_data = + pod_from_bytes::(&account_handle.data[..RecordData::WRITABLE_START_INDEX]) + .unwrap(); assert_eq!(account_data.authority, new_authority.pubkey()); assert_eq!(account_data.version, RecordData::CURRENT_VERSION); + assert_eq!( + &account_handle.data[RecordData::WRITABLE_START_INDEX..], + new_data, + ); } #[tokio::test] @@ -470,9 +453,7 @@ async fn set_authority_fail_wrong_authority() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let wrong_authority = Keypair::new(); @@ -510,9 +491,7 @@ async fn set_authority_fail_unsigned() { let authority = Keypair::new(); let account = Keypair::new(); - let data = Data { - bytes: [222u8; Data::DATA_SIZE], - }; + let data = &[222u8; 8]; initialize_storage_account(&mut context, &authority, &account, data).await; let transaction = Transaction::new_signed_with_payer( From 9376895bfd4604e5e2e1791820379931651d325d Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 09:19:42 +0900 Subject: [PATCH 3/3] update description for `RecordData` --- record/program/src/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/record/program/src/state.rs b/record/program/src/state.rs index 32c8d3157ed..c809ec0a98d 100644 --- a/record/program/src/state.rs +++ b/record/program/src/state.rs @@ -4,7 +4,7 @@ use { solana_program::{program_pack::IsInitialized, pubkey::Pubkey}, }; -/// Struct wrapping data and providing metadata +/// Header type for recorded account data #[repr(C)] #[derive(Clone, Copy, Debug, PartialEq, Pod, Zeroable)] pub struct RecordData {