From 20a1df8a5e0bd0ebe56735860bf077110aab406a Mon Sep 17 00:00:00 2001 From: Vladimir Guguiev <1524432+vovacodes@users.noreply.github.com> Date: Tue, 28 Nov 2023 23:25:11 +0100 Subject: [PATCH] feat(rent-reclamation): add member check to batch_accounts_close --- .../src/instructions/batch_create.rs | 2 + .../transaction_accounts_close.rs | 12 ++++++ sdk/multisig/idl/squads_multisig_program.json | 8 ++++ .../instructions/batchAccountsClose.ts | 7 ++++ .../src/instructions/batchAccountsClose.ts | 3 ++ sdk/multisig/src/rpc/batchAccountsClose.ts | 5 ++- .../src/transactions/batchAccountsClose.ts | 3 ++ .../suites/instructions/batchAccountsClose.ts | 37 ++++++++++++++++++- 8 files changed, 75 insertions(+), 2 deletions(-) diff --git a/programs/squads_multisig_program/src/instructions/batch_create.rs b/programs/squads_multisig_program/src/instructions/batch_create.rs index 61c29b4e..3afce40d 100644 --- a/programs/squads_multisig_program/src/instructions/batch_create.rs +++ b/programs/squads_multisig_program/src/instructions/batch_create.rs @@ -91,6 +91,8 @@ impl BatchCreate<'_> { batch.size = 0; batch.executed_transaction_index = 0; + batch.invariant()?; + // Updated last transaction index in the multisig account. multisig.transaction_index = index; diff --git a/programs/squads_multisig_program/src/instructions/transaction_accounts_close.rs b/programs/squads_multisig_program/src/instructions/transaction_accounts_close.rs index d835fd1a..e2a5fa7a 100644 --- a/programs/squads_multisig_program/src/instructions/transaction_accounts_close.rs +++ b/programs/squads_multisig_program/src/instructions/transaction_accounts_close.rs @@ -403,6 +403,9 @@ pub struct BatchAccountsClose<'info> { )] pub multisig: Account<'info, Multisig>, + /// Member of the multisig. + pub member: Signer<'info>, + #[account(mut, close = rent_collector)] pub proposal: Account<'info, Proposal>, @@ -436,6 +439,15 @@ impl BatchAccountsClose<'_> { .key(); //endregion + //region member + // Has to be a member of the `multisig`. + // This is checked to prevent potential attackers from closing the `Batch` and `Proposal` + // accounts before all `VaultBatchTransaction`s are closed. + require!( + multisig.is_member(self.member.key()).is_some(), + MultisigError::NotAMember + ); + //region rent_collector // Has to match the `multisig.rent_collector`. require_keys_eq!( diff --git a/sdk/multisig/idl/squads_multisig_program.json b/sdk/multisig/idl/squads_multisig_program.json index 0a5f736e..9f79ccd6 100644 --- a/sdk/multisig/idl/squads_multisig_program.json +++ b/sdk/multisig/idl/squads_multisig_program.json @@ -1208,6 +1208,14 @@ "isMut": false, "isSigner": false }, + { + "name": "member", + "isMut": false, + "isSigner": true, + "docs": [ + "Member of the multisig." + ] + }, { "name": "proposal", "isMut": true, diff --git a/sdk/multisig/src/generated/instructions/batchAccountsClose.ts b/sdk/multisig/src/generated/instructions/batchAccountsClose.ts index 3625cb41..cd5f6a27 100644 --- a/sdk/multisig/src/generated/instructions/batchAccountsClose.ts +++ b/sdk/multisig/src/generated/instructions/batchAccountsClose.ts @@ -23,6 +23,7 @@ export const batchAccountsCloseStruct = new beet.BeetArgsStruct<{ * Accounts required by the _batchAccountsClose_ instruction * * @property [] multisig + * @property [**signer**] member * @property [_writable_] proposal * @property [_writable_] batch * @property [_writable_] rentCollector @@ -32,6 +33,7 @@ export const batchAccountsCloseStruct = new beet.BeetArgsStruct<{ */ export type BatchAccountsCloseInstructionAccounts = { multisig: web3.PublicKey + member: web3.PublicKey proposal: web3.PublicKey batch: web3.PublicKey rentCollector: web3.PublicKey @@ -64,6 +66,11 @@ export function createBatchAccountsCloseInstruction( isWritable: false, isSigner: false, }, + { + pubkey: accounts.member, + isWritable: false, + isSigner: true, + }, { pubkey: accounts.proposal, isWritable: true, diff --git a/sdk/multisig/src/instructions/batchAccountsClose.ts b/sdk/multisig/src/instructions/batchAccountsClose.ts index a11a2830..3a07aa3c 100644 --- a/sdk/multisig/src/instructions/batchAccountsClose.ts +++ b/sdk/multisig/src/instructions/batchAccountsClose.ts @@ -14,11 +14,13 @@ import { getProposalPda, getTransactionPda } from "../pda"; */ export function batchAccountsClose({ multisigPda, + member, rentCollector, batchIndex, programId = PROGRAM_ID, }: { multisigPda: PublicKey; + member: PublicKey; rentCollector: PublicKey; batchIndex: bigint; programId?: PublicKey; @@ -37,6 +39,7 @@ export function batchAccountsClose({ return createBatchAccountsCloseInstruction( { multisig: multisigPda, + member, rentCollector, proposal: proposalPda, batch: batchPda, diff --git a/sdk/multisig/src/rpc/batchAccountsClose.ts b/sdk/multisig/src/rpc/batchAccountsClose.ts index 22d16e11..98eba18e 100644 --- a/sdk/multisig/src/rpc/batchAccountsClose.ts +++ b/sdk/multisig/src/rpc/batchAccountsClose.ts @@ -22,6 +22,7 @@ export async function batchAccountsClose({ connection, feePayer, multisigPda, + member, rentCollector, batchIndex, sendOptions, @@ -30,6 +31,7 @@ export async function batchAccountsClose({ connection: Connection; feePayer: Signer; multisigPda: PublicKey; + member: Signer; rentCollector: PublicKey; batchIndex: bigint; sendOptions?: SendOptions; @@ -40,13 +42,14 @@ export async function batchAccountsClose({ const tx = transactions.batchAccountsClose({ blockhash, feePayer: feePayer.publicKey, + member: member.publicKey, rentCollector, batchIndex, multisigPda, programId, }); - tx.sign([feePayer]); + tx.sign([feePayer, member]); try { return await connection.sendTransaction(tx, sendOptions); diff --git a/sdk/multisig/src/transactions/batchAccountsClose.ts b/sdk/multisig/src/transactions/batchAccountsClose.ts index fe48a990..f116ab8d 100644 --- a/sdk/multisig/src/transactions/batchAccountsClose.ts +++ b/sdk/multisig/src/transactions/batchAccountsClose.ts @@ -9,6 +9,7 @@ export function batchAccountsClose({ blockhash, feePayer, multisigPda, + member, rentCollector, batchIndex, programId, @@ -16,6 +17,7 @@ export function batchAccountsClose({ blockhash: string; feePayer: PublicKey; multisigPda: PublicKey; + member: PublicKey; rentCollector: PublicKey; batchIndex: bigint; programId?: PublicKey; @@ -26,6 +28,7 @@ export function batchAccountsClose({ instructions: [ instructions.batchAccountsClose({ multisigPda, + member, rentCollector, batchIndex, programId, diff --git a/tests/suites/instructions/batchAccountsClose.ts b/tests/suites/instructions/batchAccountsClose.ts index 4d949008..6b47155c 100644 --- a/tests/suites/instructions/batchAccountsClose.ts +++ b/tests/suites/instructions/batchAccountsClose.ts @@ -160,6 +160,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: Keypair.generate().publicKey, batchIndex, programId, @@ -168,6 +169,31 @@ describe("Instructions / batch_accounts_close", () => { ); }); + it("error: not a member", async () => { + const batchIndex = testMultisig.rejectedBatchIndex; + + const fakeMember = Keypair.generate(); + + const multisigAccount = await Multisig.fromAccountAddress( + connection, + multisigPda + ); + + await assert.rejects( + () => + multisig.rpc.batchAccountsClose({ + connection, + feePayer: members.almighty, + multisigPda, + member: fakeMember, + rentCollector: multisigAccount.rentCollector!, + batchIndex, + programId, + }), + /Provided pubkey is not a member of multisig/ + ); + }); + it("error: invalid rent_collector", async () => { const batchIndex = testMultisig.rejectedBatchIndex; @@ -179,6 +205,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: fakeRentCollector, batchIndex, programId, @@ -269,6 +296,7 @@ describe("Instructions / batch_accounts_close", () => { const ix = multisig.generated.createBatchAccountsCloseInstruction( { multisig: multisigPda, + member: members.almighty.publicKey, rentCollector: vaultPda, proposal: multisig.getProposalPda({ multisigPda: otherMultisig, @@ -292,7 +320,7 @@ describe("Instructions / batch_accounts_close", () => { instructions: [ix], }).compileToV0Message(); const tx = new VersionedTransaction(message); - tx.sign([feePayer]); + tx.sign([feePayer, members.almighty]); await assert.rejects( () => @@ -317,6 +345,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId, @@ -339,6 +368,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId, @@ -361,6 +391,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId, @@ -381,6 +412,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId, @@ -423,6 +455,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId, @@ -442,6 +475,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId, @@ -461,6 +495,7 @@ describe("Instructions / batch_accounts_close", () => { connection, feePayer: members.almighty, multisigPda, + member: members.almighty, rentCollector: multisigAccount.rentCollector!, batchIndex, programId,