feat(rent-reclamation): add member check to batch_accounts_close

Squads-Protocol · Nov 28, 2023 · 20a1df8 · 20a1df8
1 parent 5ce68a5
commit 20a1df8
Show file tree

Hide file tree

Showing 8 changed files with 75 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -9,13 +9,15 @@ export function batchAccountsClose({
   blockhash,
   feePayer,
   multisigPda,
+  member,
   rentCollector,
   batchIndex,
   programId,
 }: {
   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
@@ -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,