Skip to content

Commit

Permalink
update close to make it safe to call manually
Browse files Browse the repository at this point in the history
  • Loading branch information
stegaBOB committed Oct 6, 2022
1 parent b6f192d commit d984184
Show file tree
Hide file tree
Showing 9 changed files with 154 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ The minor version will be incremented upon a breaking change and the patch versi
* spl: Add `freeze_delegated_account` and `thaw_delegated_account` wrappers ([#2164](https://github.com/coral-xyz/anchor/pull/2164))
* ts: Add nested PDA inference ([#2194](https://github.com/coral-xyz/anchor/pull/2194))
* ts: Add ability to resolve missing accounts with a custom resolver ([#2194](https://github.com/coral-xyz/anchor/pull/2194))
* lang: Updates `AccountsClose` to make it safe to call manually ([#2209](https://github.com/coral-xyz/anchor/pull/2209))

### Fixes

Expand Down
9 changes: 1 addition & 8 deletions lang/src/accounts/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ impl<'info, T: AccountSerialize + AccountDeserialize + Owner + Clone> AccountsEx
{
fn exit(&self, program_id: &Pubkey) -> Result<()> {
// Only persist if the owner is the current program.
if &T::owner() == program_id {
if self.info.owner == program_id {
let info = self.to_account_info();
let mut data = info.try_borrow_mut_data()?;
let dst: &mut [u8] = &mut data;
Expand All @@ -348,13 +348,6 @@ impl<'info, T: AccountSerialize + AccountDeserialize + Owner + Clone> AccountsEx
}
}

/// This function is for INTERNAL USE ONLY.
/// Do NOT use this function in a program.
/// Manual closing of `Account<'info, T>` types is NOT supported.
///
/// Details: Using `close` with `Account<'info, T>` is not safe because
/// it requires the `mut` constraint but for that type the constraint
/// overwrites the "closed account" discriminator at the end of the instruction.
impl<'info, T: AccountSerialize + AccountDeserialize + Owner + Clone> AccountsClose<'info>
for Account<'info, T>
{
Expand Down
7 changes: 0 additions & 7 deletions lang/src/accounts/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,6 @@ impl<'info, T: ZeroCopy + Owner> AccountsExit<'info> for AccountLoader<'info, T>
}
}

/// This function is for INTERNAL USE ONLY.
/// Do NOT use this function in a program.
/// Manual closing of `AccountLoader<'info, T>` types is NOT supported.
///
/// Details: Using `close` with `AccountLoader<'info, T>` is not safe because
/// it requires the `mut` constraint but for that type the constraint
/// overwrites the "closed account" discriminator at the end of the instruction.
impl<'info, T: ZeroCopy + Owner> AccountsClose<'info> for AccountLoader<'info, T> {
fn close(&self, sol_destination: AccountInfo<'info>) -> Result<()> {
crate::common::close(self.to_account_info(), sol_destination)
Expand Down
7 changes: 0 additions & 7 deletions lang/src/accounts/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,6 @@ impl<'info, T: ZeroCopy> AccountsExit<'info> for Loader<'info, T> {
}
}

/// This function is for INTERNAL USE ONLY.
/// Do NOT use this function in a program.
/// Manual closing of `Loader<'info, T>` types is NOT supported.
///
/// Details: Using `close` with `Loader<'info, T>` is not safe because
/// it requires the `mut` constraint but for that type the constraint
/// overwrites the "closed account" discriminator at the end of the instruction.
#[allow(deprecated)]
impl<'info, T: ZeroCopy> AccountsClose<'info> for Loader<'info, T> {
fn close(&self, sol_destination: AccountInfo<'info>) -> Result<()> {
Expand Down
7 changes: 0 additions & 7 deletions lang/src/accounts/program_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,6 @@ impl<'info, T: AccountSerialize + AccountDeserialize + Clone> AccountsExit<'info
}
}

/// This function is for INTERNAL USE ONLY.
/// Do NOT use this function in a program.
/// Manual closing of `ProgramAccount<'info, T>` types is NOT supported.
///
/// Details: Using `close` with `ProgramAccount<'info, T>` is not safe because
/// it requires the `mut` constraint but for that type the constraint
/// overwrites the "closed account" discriminator at the end of the instruction.
#[allow(deprecated)]
impl<'info, T: AccountSerialize + AccountDeserialize + Clone> AccountsClose<'info>
for ProgramAccount<'info, T>
Expand Down
4 changes: 2 additions & 2 deletions lang/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ pub mod prelude {
program, require, require_eq, require_gt, require_gte, require_keys_eq, require_keys_neq,
require_neq, solana_program::bpf_loader_upgradeable::UpgradeableLoaderState, source, state,
system_program::System, zero_copy, AccountDeserialize, AccountSerialize, Accounts,
AccountsExit, AnchorDeserialize, AnchorSerialize, Id, Key, Owner, ProgramData, Result,
ToAccountInfo, ToAccountInfos, ToAccountMetas,
AccountsClose, AccountsExit, AnchorDeserialize, AnchorSerialize, Id, Key, Owner,
ProgramData, Result, ToAccountInfo, ToAccountInfos, ToAccountMetas,
};
pub use anchor_attribute_error::*;
pub use borsh;
Expand Down
16 changes: 16 additions & 0 deletions tests/misc/programs/misc/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,22 @@ pub struct TestClose<'info> {
sol_dest: AccountInfo<'info>,
}

#[derive(Accounts)]
pub struct TestCloseTwice<'info> {
#[account(mut, close = sol_dest)]
pub data: Account<'info, Data>,
/// CHECK:
pub sol_dest: AccountInfo<'info>,
}

#[derive(Accounts)]
pub struct TestCloseMut<'info> {
#[account(mut)]
pub data: Account<'info, Data>,
/// CHECK:
pub sol_dest: AccountInfo<'info>,
}

#[derive(Accounts)]
pub struct TestU16<'info> {
#[account(zero)]
Expand Down
18 changes: 18 additions & 0 deletions tests/misc/programs/misc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,24 @@ pub mod misc {
Ok(())
}

pub fn test_close_twice(ctx: Context<TestCloseTwice>) -> Result<()> {
let data_account = &ctx.accounts.data;
let sol_dest_info = ctx.accounts.sol_dest.to_account_info();
data_account.close(sol_dest_info)?;
let data_account_info: &AccountInfo = data_account.as_ref();
require_keys_eq!(*data_account_info.owner, System::id());
Ok(())
}

pub fn test_close_mut(ctx: Context<TestCloseMut>) -> Result<()> {
let data_account = &ctx.accounts.data;
let sol_dest_info = ctx.accounts.sol_dest.to_account_info();
data_account.close(sol_dest_info)?;
let data_account_info: &AccountInfo = data_account.as_ref();
require_keys_eq!(*data_account_info.owner, System::id());
Ok(())
}

pub fn test_instruction_constraint(
_ctx: Context<TestInstructionConstraint>,
_nonce: u8,
Expand Down
132 changes: 116 additions & 16 deletions tests/misc/tests/misc/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,37 +244,137 @@ describe("misc", () => {
});

it("Can close an account", async () => {
const openAccount = await program.provider.connection.getAccountInfo(
data.publicKey
);
const connection = program.provider.connection;
const openAccount = await connection.getAccountInfo(data.publicKey);

assert.isNotNull(openAccount);
const openAccountBalance = openAccount.lamports;
// double balance to calculate closed balance correctly
const transferIx = anchor.web3.SystemProgram.transfer({
fromPubkey: provider.wallet.publicKey,
toPubkey: data.publicKey,
lamports: openAccountBalance,
});
const transferTransaction = new anchor.web3.Transaction().add(transferIx);
await provider.sendAndConfirm(transferTransaction);

let beforeBalance = (
await program.provider.connection.getAccountInfo(
provider.wallet.publicKey
)
await connection.getAccountInfo(provider.wallet.publicKey)
).lamports;

await program.rpc.testClose({
accounts: {
await program.methods
.testClose()
.accounts({
data: data.publicKey,
solDest: provider.wallet.publicKey,
},
})
.postInstructions([transferIx]);

let afterBalance = (
await connection.getAccountInfo(provider.wallet.publicKey)
).lamports;

// Retrieved rent exemption sol.
expect(afterBalance > beforeBalance).to.be.true;

const closedAccount = await connection.getAccountInfo(data.publicKey);

assert.isTrue(closedAccount.data.length === 0);
assert.isTrue(closedAccount.owner.equals(SystemProgram.programId));
});

it("Can close an account twice", async () => {
const data = anchor.web3.Keypair.generate();
await program.methods
.initialize(new anchor.BN(10), new anchor.BN(10))
.accounts({ data: data.publicKey })
.preInstructions([await program.account.data.createInstruction(data)])
.signers([data])
.rpc();

const connection = program.provider.connection;
const openAccount = await connection.getAccountInfo(data.publicKey);
assert.isNotNull(openAccount);

const openAccountBalance = openAccount.lamports;
// double balance to calculate closed balance correctly
const transferIx = anchor.web3.SystemProgram.transfer({
fromPubkey: provider.wallet.publicKey,
toPubkey: data.publicKey,
lamports: openAccountBalance,
});
const transferTransaction = new anchor.web3.Transaction().add(transferIx);
await provider.sendAndConfirm(transferTransaction);

let beforeBalance = (
await connection.getAccountInfo(provider.wallet.publicKey)
).lamports;

await program.methods
.testCloseTwice()
.accounts({
data: data.publicKey,
solDest: provider.wallet.publicKey,
})
.postInstructions([transferIx]);

let afterBalance = (
await program.provider.connection.getAccountInfo(
provider.wallet.publicKey
)
await connection.getAccountInfo(provider.wallet.publicKey)
).lamports;

// Retrieved rent exemption sol.
expect(afterBalance > beforeBalance).to.be.true;

const closedAccount = await program.provider.connection.getAccountInfo(
data.publicKey
);
assert.isNull(closedAccount);
const closedAccount = await connection.getAccountInfo(data.publicKey);
assert.isTrue(closedAccount.data.length === 0);
assert.isTrue(closedAccount.owner.equals(SystemProgram.programId));
});

it("Can close a mut account manually", async () => {
const data = anchor.web3.Keypair.generate();
await program.methods
.initialize(new anchor.BN(10), new anchor.BN(10))
.accounts({ data: data.publicKey })
.preInstructions([await program.account.data.createInstruction(data)])
.signers([data])
.rpc();

const connection = program.provider.connection;
const openAccount = await connection.getAccountInfo(data.publicKey);

assert.isNotNull(openAccount);
const openAccountBalance = openAccount.lamports;
// double balance to calculate closed balance correctly
const transferIx = anchor.web3.SystemProgram.transfer({
fromPubkey: provider.wallet.publicKey,
toPubkey: data.publicKey,
lamports: openAccountBalance,
});
const transferTransaction = new anchor.web3.Transaction().add(transferIx);
await provider.sendAndConfirm(transferTransaction);

let beforeBalance = (
await connection.getAccountInfo(provider.wallet.publicKey)
).lamports;

await program.methods
.testCloseMut()
.accounts({
data: data.publicKey,
solDest: provider.wallet.publicKey,
})
.postInstructions([transferIx]);

let afterBalance = (
await connection.getAccountInfo(provider.wallet.publicKey)
).lamports;

// Retrieved rent exemption sol.
expect(afterBalance > beforeBalance).to.be.true;

const closedAccount = await connection.getAccountInfo(data.publicKey);
assert.isTrue(closedAccount.data.length === 0);
assert.isTrue(closedAccount.owner.equals(SystemProgram.programId));
});

it("Can use instruction data in accounts constraints", async () => {
Expand Down

0 comments on commit d984184

Please sign in to comment.