From ea12fea1048fb0eea7252a457e25a05590925220 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 11:41:17 +0900 Subject: [PATCH 1/5] add `Reallocate` instruction --- record/program/src/instruction.rs | 64 +++++++++++++++++++++++++++++- record/program/src/processor.rs | 66 ++++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 3 deletions(-) diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index 0c2df24cde7..769d213ec1f 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -6,6 +6,7 @@ use { instruction::{AccountMeta, Instruction}, program_error::ProgramError, pubkey::Pubkey, + system_program, }, std::mem::size_of, }; @@ -52,19 +53,36 @@ pub enum RecordInstruction<'a> { /// 1. `[signer]` Record authority /// 2. `[]` Receiver of account lamports CloseAccount, + + /// Reallocate additional space in a record account + /// + /// If the record account already has enough space to hold the specified data length, then the + /// instruction does nothing. + /// + /// Accounts expected by this instruction: + /// + /// 0. `[writable]` The record account to reallocate + /// 1. `[signer, writable]` The payer account to fund reallocation + /// 2. `[]` System program for reallocation funding + /// 3. `[signer]` The account's owner + Reallocate { + /// The length of the data to hold in the record account excluding meta data + data_length: u64, + }, } impl<'a> RecordInstruction<'a> { /// Unpacks a byte buffer into a [RecordInstruction]. pub fn unpack(input: &'a [u8]) -> Result { + const U32_BYTES: usize = 4; + const U64_BYTES: usize = 8; + let (&tag, rest) = input .split_first() .ok_or(ProgramError::InvalidInstructionData)?; 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()) @@ -84,6 +102,15 @@ impl<'a> RecordInstruction<'a> { } 2 => Self::SetAuthority, 3 => Self::CloseAccount, + 4 => { + let data_length = rest + .get(..U64_BYTES) + .and_then(|slice| slice.try_into().ok()) + .map(u64::from_le_bytes) + .ok_or(ProgramError::InvalidInstructionData)?; + + Self::Reallocate { data_length } + } _ => return Err(ProgramError::InvalidInstructionData), }) } @@ -101,6 +128,10 @@ impl<'a> RecordInstruction<'a> { } Self::SetAuthority => buf.push(2), Self::CloseAccount => buf.push(3), + Self::Reallocate { data_length } => { + buf.push(4); + buf.extend_from_slice(&data_length.to_le_bytes()); + } }; buf } @@ -160,6 +191,25 @@ pub fn close_account(record_account: &Pubkey, signer: &Pubkey, receiver: &Pubkey } } +/// Create a `RecordInstruction::Reallocate` instruction +pub fn reallocate( + record_account: &Pubkey, + payer: &Pubkey, + signer: &Pubkey, + data_length: u64, +) -> Instruction { + Instruction { + program_id: id(), + accounts: vec![ + AccountMeta::new(*record_account, false), + AccountMeta::new(*payer, true), + AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(*signer, true), + ], + data: RecordInstruction::Reallocate { data_length }.pack(), + } +} + #[cfg(test)] mod tests { use {super::*, crate::state::tests::TEST_BYTES, solana_program::program_error::ProgramError}; @@ -201,6 +251,16 @@ mod tests { assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); } + #[test] + fn serialize_reallocate() { + let data_length = 16u64; + let instruction = RecordInstruction::Reallocate { data_length }; + let mut expected = vec![4]; + expected.extend_from_slice(&data_length.to_le_bytes()); + assert_eq!(instruction.pack(), expected); + assert_eq!(RecordInstruction::unpack(&expected).unwrap(), instruction); + } + #[test] fn deserialize_invalid_instruction() { let mut expected = vec![12]; diff --git a/record/program/src/processor.rs b/record/program/src/processor.rs index 8c6c5593984..925cc59fe39 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -6,11 +6,14 @@ use { account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, msg, + program::invoke, program_error::ProgramError, program_pack::IsInitialized, pubkey::Pubkey, + system_instruction, + sysvar::{rent::Rent, Sysvar}, }, - spl_pod::bytemuck::{pod_from_bytes, pod_from_bytes_mut}, + spl_pod::bytemuck::{pod_from_bytes, pod_from_bytes_mut, pod_get_packed_len}, }; fn check_authority(authority_info: &AccountInfo, expected_authority: &Pubkey) -> ProgramResult { @@ -132,5 +135,66 @@ pub fn process_instruction( .ok_or(RecordError::Overflow)?; Ok(()) } + + RecordInstruction::Reallocate { data_length } => { + msg!("RecordInstruction::Reallocate"); + let data_info = next_account_info(account_info_iter)?; + let payer_info = next_account_info(account_info_iter)?; + let system_program_info = next_account_info(account_info_iter)?; + let authority_info = next_account_info(account_info_iter)?; + + { + let raw_data = &mut data_info.data.borrow_mut(); + 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); + } + check_authority(authority_info, &account_data.authority)?; + } + + // needed account length is the sum of the meta data length and the specified data + // length + let needed_account_length = pod_get_packed_len::() + .checked_add( + usize::try_from(data_length).map_err(|_| ProgramError::InvalidArgument)?, + ) + .unwrap(); + + // reallocate + if data_info.data_len() >= needed_account_length { + msg!("no additional reallocation needed"); + return Ok(()); + } + msg!( + "reallocating +{:?} bytes", + needed_account_length - data_info.data_len() + ); + data_info.realloc(needed_account_length, false)?; + + // if additional lamports needed to remain rent-exempt, transfer them + let current_lamport_reserve = data_info.lamports(); + let rent = Rent::get()?; + let new_rent_exempt_reserve = rent.minimum_balance(needed_account_length); + + let lamports_diff = new_rent_exempt_reserve.saturating_sub(current_lamport_reserve); + if lamports_diff > 0 { + invoke( + &system_instruction::transfer(payer_info.key, data_info.key, lamports_diff), + &[ + payer_info.clone(), + data_info.clone(), + system_program_info.clone(), + ], + )?; + } + + Ok(()) + } } } From 3efd6bd8f10f6bc4bacd09cb39516f4eaa0c7b7a Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 11:41:28 +0900 Subject: [PATCH 2/5] add tests for `Reallocate` instruction --- record/program/tests/functional.rs | 157 ++++++++++++++++++++++++++++- 1 file changed, 156 insertions(+), 1 deletion(-) diff --git a/record/program/tests/functional.rs b/record/program/tests/functional.rs index 6204c50731c..8a62dc2ed40 100644 --- a/record/program/tests/functional.rs +++ b/record/program/tests/functional.rs @@ -5,7 +5,7 @@ use { instruction::{AccountMeta, Instruction, InstructionError}, pubkey::Pubkey, rent::Rent, - system_instruction, + system_instruction, system_program, }, solana_program_test::*, solana_sdk::{ @@ -518,3 +518,158 @@ async fn set_authority_fail_unsigned() { TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature) ); } + +#[tokio::test] +async fn reallocate_success() { + let mut context = program_test().start_with_context().await; + + let authority = Keypair::new(); + let account = Keypair::new(); + let data = &[222u8; 8]; + initialize_storage_account(&mut context, &authority, &account, data).await; + + let new_data_length = 16u64; + let expected_account_data_length = RecordData::WRITABLE_START_INDEX + .checked_add(new_data_length as usize) + .unwrap(); + + let transaction = Transaction::new_signed_with_payer( + &[instruction::reallocate( + &account.pubkey(), + &context.payer.pubkey(), + &authority.pubkey(), + new_data_length, + )], + Some(&context.payer.pubkey()), + &[&context.payer, &authority], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + + let account_handle = context + .banks_client + .get_account(account.pubkey()) + .await + .unwrap() + .unwrap(); + + assert_eq!(account_handle.data.len(), expected_account_data_length); + + // reallocate to a smaller length + let old_data_length = 8u64; + let transaction = Transaction::new_signed_with_payer( + &[instruction::reallocate( + &account.pubkey(), + &context.payer.pubkey(), + &authority.pubkey(), + old_data_length, + )], + Some(&context.payer.pubkey()), + &[&context.payer, &authority], + context.last_blockhash, + ); + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + + let account = context + .banks_client + .get_account(account.pubkey()) + .await + .unwrap() + .unwrap(); + + assert_eq!(account.data.len(), expected_account_data_length); +} + +#[tokio::test] +async fn reallocate_fail_wrong_authority() { + let mut context = program_test().start_with_context().await; + + let authority = Keypair::new(); + let account = Keypair::new(); + let data = &[222u8; 8]; + initialize_storage_account(&mut context, &authority, &account, data).await; + + let new_data_length = 16u64; + + let wrong_authority = Keypair::new(); + let transaction = Transaction::new_signed_with_payer( + &[Instruction { + program_id: id(), + accounts: vec![ + AccountMeta::new(account.pubkey(), false), + AccountMeta::new(context.payer.pubkey(), true), + AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new(wrong_authority.pubkey(), true), + ], + data: instruction::RecordInstruction::Reallocate { + data_length: new_data_length, + } + .pack(), + }], + Some(&context.payer.pubkey()), + &[&context.payer, &wrong_authority], + context.last_blockhash, + ); + + assert_eq!( + context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(), + TransactionError::InstructionError( + 0, + InstructionError::Custom(RecordError::IncorrectAuthority as u32) + ) + ); +} + +#[tokio::test] +async fn reallocate_fail_unsigned() { + let mut context = program_test().start_with_context().await; + + let authority = Keypair::new(); + let account = Keypair::new(); + let data = &[222u8; 8]; + initialize_storage_account(&mut context, &authority, &account, data).await; + + let new_data_length = 16u64; + + let transaction = Transaction::new_signed_with_payer( + &[Instruction { + program_id: id(), + accounts: vec![ + AccountMeta::new(account.pubkey(), false), + AccountMeta::new(context.payer.pubkey(), true), + AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new(authority.pubkey(), false), + ], + data: instruction::RecordInstruction::Reallocate { + data_length: new_data_length, + } + .pack(), + }], + Some(&context.payer.pubkey()), + &[&context.payer], + context.last_blockhash, + ); + + assert_eq!( + context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(), + TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature) + ); +} From afd2b012a7a641e99546e59635929a0b66fa9035 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 11:48:58 +0900 Subject: [PATCH 3/5] cargo fmt --- record/program/src/instruction.rs | 7 ++++--- record/program/src/processor.rs | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index 769d213ec1f..fe95ca0e531 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -56,8 +56,8 @@ pub enum RecordInstruction<'a> { /// Reallocate additional space in a record account /// - /// If the record account already has enough space to hold the specified data length, then the - /// instruction does nothing. + /// If the record account already has enough space to hold the specified + /// data length, then the instruction does nothing. /// /// Accounts expected by this instruction: /// @@ -66,7 +66,8 @@ pub enum RecordInstruction<'a> { /// 2. `[]` System program for reallocation funding /// 3. `[signer]` The account's owner Reallocate { - /// The length of the data to hold in the record account excluding meta data + /// The length of the data to hold in the record account excluding meta + /// data data_length: u64, }, } diff --git a/record/program/src/processor.rs b/record/program/src/processor.rs index 925cc59fe39..e2e29ef85bb 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -158,8 +158,8 @@ pub fn process_instruction( check_authority(authority_info, &account_data.authority)?; } - // needed account length is the sum of the meta data length and the specified data - // length + // needed account length is the sum of the meta data length and the specified + // data length let needed_account_length = pod_get_packed_len::() .checked_add( usize::try_from(data_length).map_err(|_| ProgramError::InvalidArgument)?, From d3ec7320d215da9aa954dc35e328eeefbc137238 Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Fri, 5 Jan 2024 11:51:34 +0900 Subject: [PATCH 4/5] clippy --- record/program/src/processor.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/record/program/src/processor.rs b/record/program/src/processor.rs index e2e29ef85bb..6c49bf46493 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -173,7 +173,9 @@ pub fn process_instruction( } msg!( "reallocating +{:?} bytes", - needed_account_length - data_info.data_len() + needed_account_length + .checked_sub(data_info.data_len()) + .unwrap(), ); data_info.realloc(needed_account_length, false)?; From be4f5273c283d8d9dde2c98198116a2568972eab Mon Sep 17 00:00:00 2001 From: samkim-crypto Date: Sat, 6 Jan 2024 09:25:35 +0900 Subject: [PATCH 5/5] remove lamport transfer logic --- record/program/src/instruction.rs | 14 +---- record/program/src/processor.rs | 23 -------- record/program/tests/functional.rs | 89 ++++++++++++++++++------------ 3 files changed, 57 insertions(+), 69 deletions(-) diff --git a/record/program/src/instruction.rs b/record/program/src/instruction.rs index fe95ca0e531..93d4f3ea946 100644 --- a/record/program/src/instruction.rs +++ b/record/program/src/instruction.rs @@ -6,7 +6,6 @@ use { instruction::{AccountMeta, Instruction}, program_error::ProgramError, pubkey::Pubkey, - system_program, }, std::mem::size_of, }; @@ -62,9 +61,7 @@ pub enum RecordInstruction<'a> { /// Accounts expected by this instruction: /// /// 0. `[writable]` The record account to reallocate - /// 1. `[signer, writable]` The payer account to fund reallocation - /// 2. `[]` System program for reallocation funding - /// 3. `[signer]` The account's owner + /// 1. `[signer]` The account's owner Reallocate { /// The length of the data to hold in the record account excluding meta /// data @@ -193,18 +190,11 @@ pub fn close_account(record_account: &Pubkey, signer: &Pubkey, receiver: &Pubkey } /// Create a `RecordInstruction::Reallocate` instruction -pub fn reallocate( - record_account: &Pubkey, - payer: &Pubkey, - signer: &Pubkey, - data_length: u64, -) -> Instruction { +pub fn reallocate(record_account: &Pubkey, signer: &Pubkey, data_length: u64) -> Instruction { Instruction { program_id: id(), accounts: vec![ AccountMeta::new(*record_account, false), - AccountMeta::new(*payer, true), - AccountMeta::new_readonly(system_program::id(), false), AccountMeta::new_readonly(*signer, true), ], data: RecordInstruction::Reallocate { data_length }.pack(), diff --git a/record/program/src/processor.rs b/record/program/src/processor.rs index 6c49bf46493..4d626cc7b96 100644 --- a/record/program/src/processor.rs +++ b/record/program/src/processor.rs @@ -6,12 +6,9 @@ use { account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, msg, - program::invoke, program_error::ProgramError, program_pack::IsInitialized, pubkey::Pubkey, - system_instruction, - sysvar::{rent::Rent, Sysvar}, }, spl_pod::bytemuck::{pod_from_bytes, pod_from_bytes_mut, pod_get_packed_len}, }; @@ -139,8 +136,6 @@ pub fn process_instruction( RecordInstruction::Reallocate { data_length } => { msg!("RecordInstruction::Reallocate"); let data_info = next_account_info(account_info_iter)?; - let payer_info = next_account_info(account_info_iter)?; - let system_program_info = next_account_info(account_info_iter)?; let authority_info = next_account_info(account_info_iter)?; { @@ -178,24 +173,6 @@ pub fn process_instruction( .unwrap(), ); data_info.realloc(needed_account_length, false)?; - - // if additional lamports needed to remain rent-exempt, transfer them - let current_lamport_reserve = data_info.lamports(); - let rent = Rent::get()?; - let new_rent_exempt_reserve = rent.minimum_balance(needed_account_length); - - let lamports_diff = new_rent_exempt_reserve.saturating_sub(current_lamport_reserve); - if lamports_diff > 0 { - invoke( - &system_instruction::transfer(payer_info.key, data_info.key, lamports_diff), - &[ - payer_info.clone(), - data_info.clone(), - system_program_info.clone(), - ], - )?; - } - Ok(()) } } diff --git a/record/program/tests/functional.rs b/record/program/tests/functional.rs index 8a62dc2ed40..4f4d325f286 100644 --- a/record/program/tests/functional.rs +++ b/record/program/tests/functional.rs @@ -5,7 +5,7 @@ use { instruction::{AccountMeta, Instruction, InstructionError}, pubkey::Pubkey, rent::Rent, - system_instruction, system_program, + system_instruction, }, solana_program_test::*, solana_sdk::{ @@ -533,13 +533,19 @@ async fn reallocate_success() { .checked_add(new_data_length as usize) .unwrap(); + let delta_account_data_length = new_data_length.saturating_sub(data.len() as u64); + let additional_lamports_needed = + Rent::default().minimum_balance(delta_account_data_length as usize); + let transaction = Transaction::new_signed_with_payer( - &[instruction::reallocate( - &account.pubkey(), - &context.payer.pubkey(), - &authority.pubkey(), - new_data_length, - )], + &[ + instruction::reallocate(&account.pubkey(), &authority.pubkey(), new_data_length), + system_instruction::transfer( + &context.payer.pubkey(), + &account.pubkey(), + additional_lamports_needed, + ), + ], Some(&context.payer.pubkey()), &[&context.payer, &authority], context.last_blockhash, @@ -564,7 +570,6 @@ async fn reallocate_success() { let transaction = Transaction::new_signed_with_payer( &[instruction::reallocate( &account.pubkey(), - &context.payer.pubkey(), &authority.pubkey(), old_data_length, )], @@ -598,22 +603,30 @@ async fn reallocate_fail_wrong_authority() { initialize_storage_account(&mut context, &authority, &account, data).await; let new_data_length = 16u64; + let delta_account_data_length = new_data_length.saturating_sub(data.len() as u64); + let additional_lamports_needed = + Rent::default().minimum_balance(delta_account_data_length as usize); let wrong_authority = Keypair::new(); let transaction = Transaction::new_signed_with_payer( - &[Instruction { - program_id: id(), - accounts: vec![ - AccountMeta::new(account.pubkey(), false), - AccountMeta::new(context.payer.pubkey(), true), - AccountMeta::new_readonly(system_program::id(), false), - AccountMeta::new(wrong_authority.pubkey(), true), - ], - data: instruction::RecordInstruction::Reallocate { - data_length: new_data_length, - } - .pack(), - }], + &[ + Instruction { + program_id: id(), + accounts: vec![ + AccountMeta::new(account.pubkey(), false), + AccountMeta::new(wrong_authority.pubkey(), true), + ], + data: instruction::RecordInstruction::Reallocate { + data_length: new_data_length, + } + .pack(), + }, + system_instruction::transfer( + &context.payer.pubkey(), + &account.pubkey(), + additional_lamports_needed, + ), + ], Some(&context.payer.pubkey()), &[&context.payer, &wrong_authority], context.last_blockhash, @@ -643,21 +656,29 @@ async fn reallocate_fail_unsigned() { initialize_storage_account(&mut context, &authority, &account, data).await; let new_data_length = 16u64; + let delta_account_data_length = new_data_length.saturating_sub(data.len() as u64); + let additional_lamports_needed = + Rent::default().minimum_balance(delta_account_data_length as usize); let transaction = Transaction::new_signed_with_payer( - &[Instruction { - program_id: id(), - accounts: vec![ - AccountMeta::new(account.pubkey(), false), - AccountMeta::new(context.payer.pubkey(), true), - AccountMeta::new_readonly(system_program::id(), false), - AccountMeta::new(authority.pubkey(), false), - ], - data: instruction::RecordInstruction::Reallocate { - data_length: new_data_length, - } - .pack(), - }], + &[ + Instruction { + program_id: id(), + accounts: vec![ + AccountMeta::new(account.pubkey(), false), + AccountMeta::new(authority.pubkey(), false), + ], + data: instruction::RecordInstruction::Reallocate { + data_length: new_data_length, + } + .pack(), + }, + system_instruction::transfer( + &context.payer.pubkey(), + &account.pubkey(), + additional_lamports_needed, + ), + ], Some(&context.payer.pubkey()), &[&context.payer], context.last_blockhash,