diff --git a/basics/close-account/native/program/src/instructions/close_user.rs b/basics/close-account/native/program/src/instructions/close_user.rs index 4118862b7..4d1b3cf6e 100644 --- a/basics/close-account/native/program/src/instructions/close_user.rs +++ b/basics/close-account/native/program/src/instructions/close_user.rs @@ -4,8 +4,6 @@ use solana_program::{ entrypoint::ProgramResult, program_error::ProgramError, pubkey::Pubkey, - rent::Rent, - sysvar::Sysvar, }; pub fn close_user(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramResult { @@ -26,20 +24,21 @@ pub fn close_user(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramResul return Err(ProgramError::IncorrectProgramId); } - let account_span = 0usize; - let lamports_required = (Rent::get()?).minimum_balance(account_span); - - let diff = target_account.lamports() - lamports_required; + if system_program.key != &solana_system_interface::program::ID { + return Err(ProgramError::IncorrectProgramId); + } - // Send the rent back to the payer - **target_account.lamports.borrow_mut() -= diff; - **payer.lamports.borrow_mut() += diff; + // Send all the lamports back to the payer; a zero-lamport account is + // deleted by the runtime, so the PDA can be created again later. + let lamports = target_account.lamports(); + **target_account.lamports.borrow_mut() = 0; + **payer.lamports.borrow_mut() += lamports; // Realloc the account to zero - target_account.resize(account_span)?; + target_account.resize(0)?; // Assign the account to the System Program - target_account.assign(system_program.key); + target_account.assign(&solana_system_interface::program::ID); Ok(()) } diff --git a/basics/close-account/native/program/tests/test.rs b/basics/close-account/native/program/tests/test.rs index 779bd6010..fd501c19d 100644 --- a/basics/close-account/native/program/tests/test.rs +++ b/basics/close-account/native/program/tests/test.rs @@ -8,6 +8,9 @@ use solana_transaction::Transaction; use close_account_native_program::processor::MyInstruction; +// LiteSVM's default fee for a single-signature transaction. +const TX_FEE: u64 = 5000; + #[test] fn test_close_account() { let mut svm = LiteSVM::new(); @@ -40,7 +43,31 @@ fn test_close_account() { assert!(svm.send_transaction(tx).is_ok()); - // clsose user ix + // close user ix with a bogus system program account + let bogus_program = Pubkey::new_unique(); + let data = borsh::to_vec(&MyInstruction::CloseUser).unwrap(); + + let ix = Instruction { + program_id, + accounts: vec![ + AccountMeta::new(test_account_pubkey, false), + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(bogus_program, false), + ], + data, + }; + + let tx = Transaction::new_signed_with_payer(&[ix], Some(&payer.pubkey()), &[&payer], svm.latest_blockhash()); + + let err = svm.send_transaction(tx).expect_err("expected the bogus system program to be rejected").err; + assert!(format!("{err:?}").contains("IncorrectProgramId"), "unexpected error: {err:?}"); + assert_eq!(svm.get_account(&test_account_pubkey).unwrap().owner, program_id); + + // close user ix + let payer_balance_before = svm.get_balance(&payer.pubkey()).unwrap(); + let account_balance_before = svm.get_balance(&test_account_pubkey).unwrap(); + assert!(account_balance_before > 0); + let data = borsh::to_vec(&MyInstruction::CloseUser).unwrap(); let ix = Instruction { @@ -56,4 +83,31 @@ fn test_close_account() { let tx = Transaction::new_signed_with_payer(&[ix], Some(&payer.pubkey()), &[&payer], svm.latest_blockhash()); assert!(svm.send_transaction(tx).is_ok()); + + // Closing drains every lamport back to the payer and deletes the account. + assert!(svm.get_account(&test_account_pubkey).is_none(), "expected the closed account to no longer exist"); + assert_eq!(svm.get_balance(&payer.pubkey()).unwrap(), payer_balance_before + account_balance_before - TX_FEE); + + // re-create user ix after closing + let data = borsh::to_vec(&MyInstruction::CreateUser(User { name: "Jacob".to_string() })).unwrap(); + + let ix = Instruction { + program_id, + accounts: vec![ + AccountMeta::new(test_account_pubkey, false), + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(solana_system_interface::program::ID, false), + ], + data, + }; + + svm.expire_blockhash(); + let tx = Transaction::new_signed_with_payer(&[ix], Some(&payer.pubkey()), &[&payer], svm.latest_blockhash()); + + let res = svm.send_transaction(tx); + assert!(res.is_ok(), "expected the account to be re-creatable after closing: {res:?}"); + + let account = svm.get_account(&test_account_pubkey).unwrap(); + assert_eq!(account.owner, program_id); + assert_eq!(borsh::from_slice::(&account.data).unwrap().name, "Jacob"); } diff --git a/basics/close-account/native/tests/close-account.test.ts b/basics/close-account/native/tests/close-account.test.ts index b324ab542..4b2a11343 100644 --- a/basics/close-account/native/tests/close-account.test.ts +++ b/basics/close-account/native/tests/close-account.test.ts @@ -1,4 +1,5 @@ import { + AccountRole, type Address, appendTransactionMessageInstruction, createTransactionMessage, @@ -11,11 +12,13 @@ import { setTransactionMessageFeePayerSigner, signTransactionMessageWithSigners, } from '@solana/kit'; -import { SYSTEM_PROGRAM_ADDRESS } from '@solana-program/system'; import { assert } from 'chai'; import { FailedTransactionMetadata, LiteSVM } from 'litesvm'; import { createCloseUserInstruction, createCreateUserInstruction, userDecoder } from '../ts'; +// LiteSVM's default fee for a single-signature transaction. +const TX_FEE = 5000n; + describe('Close Account!', () => { const svm = new LiteSVM(); const userName = 'Jacob'; @@ -82,7 +85,61 @@ describe('Close Account!', () => { ); }); + it('Close with a bogus system program account is rejected', async () => { + // Use a separate user so this test cannot disturb the main account. + const other = await generateKeyPairSigner(); + svm.airdrop(other.address, lamports(1_000_000_000n)); + const [otherAccountAddress] = await getProgramDerivedAddress({ + programAddress: programId, + seeds: ['USER', getAddressEncoder().encode(other.address)], + }); + + const createIx = createCreateUserInstruction(otherAccountAddress, other, programId, userName); + const createTx = await signTransactionMessageWithSigners( + pipe( + createTransactionMessage({ version: 0 }), + m => setTransactionMessageFeePayerSigner(other, m), + m => svm.setTransactionMessageLifetimeUsingLatestBlockhash(m), + m => appendTransactionMessageInstruction(createIx, m), + ), + ); + const createResult = svm.sendTransaction(createTx); + assert(!(createResult instanceof FailedTransactionMetadata), `transaction failed: ${createResult.toString()}`); + + const bogusProgram = (await generateKeyPairSigner()).address; + const closeIx = createCloseUserInstruction(otherAccountAddress, other, programId); + const ix = { + ...closeIx, + accounts: [closeIx.accounts[0], closeIx.accounts[1], { address: bogusProgram, role: AccountRole.READONLY }], + }; + + const transactionMessage = pipe( + createTransactionMessage({ version: 0 }), + m => setTransactionMessageFeePayerSigner(other, m), + m => svm.setTransactionMessageLifetimeUsingLatestBlockhash(m), + m => appendTransactionMessageInstruction(ix, m), + ); + const signedTx = await signTransactionMessageWithSigners(transactionMessage); + + const result = svm.sendTransaction(signedTx); + assert(result instanceof FailedTransactionMetadata, 'expected the bogus system program to be rejected'); + assert.include( + result.err().toString(), + 'IncorrectProgramId', + `expected the bogus system program account to be rejected, got: ${result.toString()}`, + ); + + const account = svm.getAccount(otherAccountAddress); + assert(account.exists, 'expected the user account to be untouched'); + assert.equal(account.programAddress, programId); + assert.equal(userDecoder.decode(account.data).name, userName); + }); + it('Close the account', async () => { + const payerBalanceBefore = svm.getBalance(payer.address)!; + const accountBalanceBefore = svm.getBalance(testAccountAddress)!; + assert(accountBalanceBefore > 0n, 'expected the user account to hold rent lamports'); + const ix = createCloseUserInstruction(testAccountAddress, payer, programId); const transactionMessage = pipe( @@ -96,11 +153,36 @@ describe('Close Account!', () => { const result = svm.sendTransaction(signedTx); assert(!(result instanceof FailedTransactionMetadata), `transaction failed: ${result.toString()}`); - // Closing resizes the account to zero and hands ownership back to the - // System Program, leaving only the rent-exempt minimum for a 0-byte account. + // Closing must drain every lamport back to the payer so the account + // is deleted by the runtime, not left as a rent-exempt empty shell. const account = svm.getAccount(testAccountAddress); - assert(account.exists); - assert.equal(account.programAddress, SYSTEM_PROGRAM_ADDRESS); - assert.equal(account.data.length, 0); + assert(!account.exists, 'expected the closed account to no longer exist'); + assert.equal(svm.getBalance(testAccountAddress) ?? 0n, 0n); + assert.equal( + svm.getBalance(payer.address), + payerBalanceBefore + accountBalanceBefore - TX_FEE, + 'expected the payer to receive every lamport from the closed account', + ); + }); + + it('Re-create the account after closing it', async () => { + const ix = createCreateUserInstruction(testAccountAddress, payer, programId, userName); + + svm.expireBlockhash(); + const transactionMessage = pipe( + createTransactionMessage({ version: 0 }), + m => setTransactionMessageFeePayerSigner(payer, m), + m => svm.setTransactionMessageLifetimeUsingLatestBlockhash(m), + m => appendTransactionMessageInstruction(ix, m), + ); + const signedTx = await signTransactionMessageWithSigners(transactionMessage); + + const result = svm.sendTransaction(signedTx); + assert(!(result instanceof FailedTransactionMetadata), `transaction failed: ${result.toString()}`); + + const account = svm.getAccount(testAccountAddress); + assert(account.exists, 'expected the user account to be re-created'); + assert.equal(account.programAddress, programId); + assert.equal(userDecoder.decode(account.data).name, userName); }); }); diff --git a/basics/close-account/pinocchio/program/src/lib.rs b/basics/close-account/pinocchio/program/src/lib.rs index 0500a1d80..c97a339d8 100644 --- a/basics/close-account/pinocchio/program/src/lib.rs +++ b/basics/close-account/pinocchio/program/src/lib.rs @@ -1,6 +1,5 @@ #![no_std] -use pinocchio::Resize; use pinocchio::{ cpi::{Seed, Signer}, entrypoint, @@ -82,21 +81,14 @@ fn process_close(program_id: &Address, accounts: &mut [AccountView]) -> ProgramR return Err(ProgramError::IncorrectProgramId); } - let rent = Rent::get()?; - - let account_span = 0usize; - let lamports_required = rent.try_minimum_balance(account_span)?; - - let diff = target_account.lamports() - lamports_required; - - target_account.set_lamports(target_account.lamports() - diff); - payer.set_lamports(payer.lamports() + diff); - - target_account.resize(account_span)?; - - unsafe { - target_account.assign(system_program.address()); + if system_program.address() != &pinocchio_system::ID { + return Err(ProgramError::IncorrectProgramId); } + // Send all the lamports back to the payer, then close the account so the + // runtime deletes it and the PDA can be created again later. + payer.set_lamports(payer.lamports() + target_account.lamports()); + target_account.close()?; + Ok(()) } diff --git a/basics/close-account/pinocchio/program/tests/tests.rs b/basics/close-account/pinocchio/program/tests/tests.rs index 0ea9916ef..7e7157362 100644 --- a/basics/close-account/pinocchio/program/tests/tests.rs +++ b/basics/close-account/pinocchio/program/tests/tests.rs @@ -9,6 +9,9 @@ use solana_transaction_error::TransactionError; use close_account_pinocchio_program::{User, CLOSE_DISCRIMINATOR, CREATE_DISCRIMINATOR}; +// LiteSVM's default fee for a single-signature transaction. +const TX_FEE: u64 = 5000; + #[test] fn test_close_account() { let mut svm = LiteSVM::new(); @@ -34,6 +37,7 @@ fn test_close_account() { let name_len = b"Jacob".len().min(User::LEN); name[..name_len].copy_from_slice(&b"Jacob"[..name_len]); data.extend_from_slice(&name); + let create_data = data.clone(); let ix = Instruction { program_id, @@ -90,7 +94,29 @@ fn test_close_account() { "expected the attacker's target PDA to be rejected as not belonging to them" ); + // process_close with a bogus system program account + let bogus_program = Pubkey::new_unique(); + let ix = Instruction { + program_id, + accounts: vec![ + AccountMeta::new(test_account_pubkey, false), + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(bogus_program, false), + ], + data: vec![CLOSE_DISCRIMINATOR], + }; + + let tx = Transaction::new_signed_with_payer(&[ix], Some(&payer.pubkey()), &[&payer], svm.latest_blockhash()); + + let err = svm.send_transaction(tx).expect_err("expected the bogus system program to be rejected").err; + assert_eq!(err, TransactionError::InstructionError(0, InstructionError::IncorrectProgramId)); + assert_eq!(svm.get_account(&test_account_pubkey).unwrap().owner, program_id); + // process_close + let payer_balance_before = svm.get_balance(&payer.pubkey()).unwrap(); + let account_balance_before = svm.get_balance(&test_account_pubkey).unwrap(); + assert!(account_balance_before > 0); + let mut data = Vec::new(); data.push(CLOSE_DISCRIMINATOR); @@ -109,7 +135,29 @@ fn test_close_account() { let res = svm.send_transaction(tx); assert!(res.is_ok()); + // Closing drains every lamport back to the payer and deletes the account. + assert!(svm.get_account(&test_account_pubkey).is_none(), "expected the closed account to no longer exist"); + assert_eq!(svm.get_balance(&payer.pubkey()).unwrap(), payer_balance_before + account_balance_before - TX_FEE); + + // process_user again after closing + let ix = Instruction { + program_id, + accounts: vec![ + AccountMeta::new(test_account_pubkey, false), + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(solana_system_interface::program::ID, false), + ], + data: create_data, + }; + + svm.expire_blockhash(); + let tx = Transaction::new_signed_with_payer(&[ix], Some(&payer.pubkey()), &[&payer], svm.latest_blockhash()); + + let res = svm.send_transaction(tx); + assert!(res.is_ok(), "expected the account to be re-creatable after closing: {res:?}"); + let account = svm.get_account(&test_account_pubkey).unwrap(); - assert_eq!(account.data.len(), 0); - assert_eq!(account.owner, solana_system_interface::program::ID); + assert_eq!(account.data.len(), User::LEN); + assert_eq!(account.owner, program_id); + assert_eq!(&account.data[..5], b"Jacob"); } diff --git a/basics/close-account/pinocchio/tests/close-account.test.ts b/basics/close-account/pinocchio/tests/close-account.test.ts index d5316a476..be1020c00 100644 --- a/basics/close-account/pinocchio/tests/close-account.test.ts +++ b/basics/close-account/pinocchio/tests/close-account.test.ts @@ -37,6 +37,9 @@ const createUserEncoder = getStructEncoder([ const userNameDecoder = fixDecoderSize(getUtf8Decoder(), USER_ACCOUNT_SIZE); +// LiteSVM's default fee for a single-signature transaction. +const TX_FEE = 5000n; + describe('Close Account!', () => { const svm = new LiteSVM(); let programId: Address; @@ -64,10 +67,10 @@ describe('Close Account!', () => { ]; }); - async function sendInstruction(ix: Instruction) { + async function sendInstruction(ix: Instruction, feePayer: KeyPairSigner = payer) { const transactionMessage = pipe( createTransactionMessage({ version: 0 }), - m => setTransactionMessageFeePayerSigner(payer, m), + m => setTransactionMessageFeePayerSigner(feePayer, m), m => svm.setTransactionMessageLifetimeUsingLatestBlockhash(m), m => appendTransactionMessageInstruction(ix, m), ); @@ -92,8 +95,59 @@ describe('Close Account!', () => { assert.equal(userNameDecoder.decode(account.data), 'Jacob'); }); + it('Close with a bogus system program account is rejected', async () => { + // Use a separate user so this test cannot disturb the main account. + const other = await generateKeyPairSigner(); + svm.airdrop(other.address, lamports(10_000_000_000n)); + const [otherAccount, otherBump] = await getProgramDerivedAddress({ + programAddress: programId, + seeds: ['USER', getAddressEncoder().encode(other.address)], + }); + + const createResult = await sendInstruction( + { + programAddress: programId, + accounts: [ + { address: otherAccount, role: AccountRole.WRITABLE }, + { address: other.address, role: AccountRole.WRITABLE_SIGNER, signer: other }, + { address: SYSTEM_PROGRAM_ADDRESS, role: AccountRole.READONLY }, + ], + data: createUserEncoder.encode({ discriminator: CREATE_DISCRIMINATOR, bump: otherBump, name: 'Jacob' }), + }, + other, + ); + assert(!(createResult instanceof FailedTransactionMetadata), `transaction failed: ${createResult.toString()}`); + + const bogusProgram = (await generateKeyPairSigner()).address; + const result = await sendInstruction( + { + programAddress: programId, + accounts: [ + { address: otherAccount, role: AccountRole.WRITABLE }, + { address: other.address, role: AccountRole.WRITABLE_SIGNER, signer: other }, + { address: bogusProgram, role: AccountRole.READONLY }, + ], + data: new Uint8Array([CLOSE_DISCRIMINATOR]), + }, + other, + ); + assert(result instanceof FailedTransactionMetadata, 'expected the bogus system program to be rejected'); + assert.include( + result.err().toString(), + 'IncorrectProgramId', + `expected the bogus system program account to be rejected, got: ${result.toString()}`, + ); + + const account = svm.getAccount(otherAccount); + assert(account.exists, 'expected the user account to be untouched'); + assert.equal(account.programAddress, programId); + assert.equal(userNameDecoder.decode(account.data), 'Jacob'); + }); + it('Close the account', async () => { const payerBalanceBefore = svm.getBalance(payer.address)!; + const accountBalanceBefore = svm.getBalance(userAccount)!; + assert(accountBalanceBefore > 0n, 'expected the user account to hold rent lamports'); const ix = { programAddress: programId, @@ -104,16 +158,32 @@ describe('Close Account!', () => { const result = await sendInstruction(ix); assert(!(result instanceof FailedTransactionMetadata), `transaction failed: ${result.toString()}`); + // Closing must drain every lamport back to the payer so the account + // is deleted by the runtime, not left as a rent-exempt empty shell. const account = svm.getAccount(userAccount); - assert(account.exists, 'expected account to still exist with zeroed data'); - assert.equal(account.data.length, 0); + assert(!account.exists, 'expected the closed account to no longer exist'); + assert.equal(svm.getBalance(userAccount) ?? 0n, 0n); assert.equal( - account.programAddress, - SYSTEM_PROGRAM_ADDRESS, - 'expected account to be reassigned to the system program', + svm.getBalance(payer.address), + payerBalanceBefore + accountBalanceBefore - TX_FEE, + 'expected the payer to receive every lamport from the closed account', ); + }); - const payerBalanceAfter = svm.getBalance(payer.address)!; - assert(payerBalanceAfter > payerBalanceBefore, 'expected payer to reclaim the rent lamports'); + it('Re-create the account after closing it', async () => { + const ix = { + programAddress: programId, + accounts: keys, + data: createUserEncoder.encode({ discriminator: CREATE_DISCRIMINATOR, bump, name: 'Jacob' }), + }; + + svm.expireBlockhash(); + const result = await sendInstruction(ix); + assert(!(result instanceof FailedTransactionMetadata), `transaction failed: ${result.toString()}`); + + const account = svm.getAccount(userAccount); + assert(account.exists, 'expected the user account to be re-created'); + assert.equal(account.programAddress, programId); + assert.equal(userNameDecoder.decode(account.data), 'Jacob'); }); });