feat(config_transaction_accounts_close,vault_transaction_accounts_clo…

…se): allow closing stale tx accounts with no proposal

programs/squads_multisig_program/src/instructions/transaction_accounts_close.rs

@@ -13,6 +13,7 @@ use anchor_lang::prelude::*;
 
 use crate::errors::*;
 use crate::state::*;
+use crate::utils;
 
 #[derive(Accounts)]
 pub struct ConfigTransactionAccountsClose<'info> {
@@ -23,18 +24,25 @@ pub struct ConfigTransactionAccountsClose<'info> {
     )]
     pub multisig: Account<'info, Multisig>,
 
+    /// CHECK: `seeds` and `bump` verify that the account is the canonical Proposal,
+    ///         the logic within `config_transaction_accounts_close` does the rest of the checks.
     #[account(
         mut,
-        has_one = multisig @ MultisigError::ProposalForAnotherMultisig,
-        close = rent_collector
+        seeds = [
+            SEED_PREFIX,
+            multisig.key().as_ref(),
+            SEED_TRANSACTION,
+            &transaction.index.to_le_bytes(),
+            SEED_PROPOSAL,
+        ],
+        bump,
     )]
-    pub proposal: Account<'info, Proposal>,
+    pub proposal: AccountInfo<'info>,
 
     /// ConfigTransaction corresponding to the `proposal`.
     #[account(
         mut,
         has_one = multisig @ MultisigError::TransactionForAnotherMultisig,
-        constraint = transaction.index == proposal.transaction_index @ MultisigError::TransactionNotMatchingProposal,
         close = rent_collector
     )]
     pub transaction: Account<'info, ConfigTransaction>,
@@ -51,47 +59,63 @@ pub struct ConfigTransactionAccountsClose<'info> {
 }
 
 impl ConfigTransactionAccountsClose<'_> {
-    fn validate(&self) -> Result<()> {
-        let Self {
-            multisig, proposal, ..
-        } = self;
+    /// Closes a `ConfigTransaction` and the corresponding `Proposal`.
+    /// `transaction` can be closed if either:
+    /// - the `proposal` is in a terminal state: `Executed`, `Rejected`, or `Cancelled`.
+    /// - the `proposal` is stale.
+    pub fn config_transaction_accounts_close(ctx: Context<Self>) -> Result<()> {
+        let multisig = &ctx.accounts.multisig;
+        let transaction = &ctx.accounts.transaction;
+        let proposal = &mut ctx.accounts.proposal;
+        let rent_collector = &ctx.accounts.rent_collector;
 
-        let is_stale = proposal.transaction_index <= multisig.stale_transaction_index;
+        let is_stale = transaction.index <= multisig.stale_transaction_index;
+
+        let proposal_account = if proposal.data.borrow().is_empty() {
+            None
+        } else {
+            Some(Proposal::try_deserialize(
+                &mut &**proposal.data.borrow_mut(),
+            )?)
+        };
 
-        // Has to be either stale or in a terminal state.
         #[allow(deprecated)]
-        let can_close = match proposal.status {
-            // Draft proposals can only be closed if stale,
-            // so they can't be activated anymore.
-            ProposalStatus::Draft { .. } => is_stale,
-            // Active proposals can only be closed if stale,
-            // so they can't be voted on anymore.
-            ProposalStatus::Active { .. } => is_stale,
-            // Approved proposals for ConfigTransactions can be closed if stale,
-            // because they cannot be executed anymore.
-            ProposalStatus::Approved { .. } => is_stale,
-            // Rejected proposals can be closed.
-            ProposalStatus::Rejected { .. } => true,
-            // Executed proposals can be closed.
-            ProposalStatus::Executed { .. } => true,
-            // Cancelled proposals can be closed.
-            ProposalStatus::Cancelled { .. } => true,
-            // Should never really be in this state.
-            ProposalStatus::Executing => false,
+        let can_close = if let Some(proposal_account) = &proposal_account {
+            match proposal_account.status {
+                // Draft proposals can only be closed if stale,
+                // so they can't be activated anymore.
+                ProposalStatus::Draft { .. } => is_stale,
+                // Active proposals can only be closed if stale,
+                // so they can't be voted on anymore.
+                ProposalStatus::Active { .. } => is_stale,
+                // Approved proposals for ConfigTransactions can be closed if stale,
+                // because they cannot be executed anymore.
+                ProposalStatus::Approved { .. } => is_stale,
+                // Rejected proposals can be closed.
+                ProposalStatus::Rejected { .. } => true,
+                // Executed proposals can be closed.
+                ProposalStatus::Executed { .. } => true,
+                // Cancelled proposals can be closed.
+                ProposalStatus::Cancelled { .. } => true,
+                // Should never really be in this state.
+                ProposalStatus::Executing => false,
+            }
+        } else {
+            // If no Proposal account exists then the ConfigTransaction can only be closed if stale
+            is_stale
         };
 
         require!(can_close, MultisigError::InvalidProposalStatus);
 
-        Ok(())
-    }
+        // Close the `proposal` account if exists.
+        if proposal_account.is_some() {
+            utils::close(
+                ctx.accounts.proposal.to_account_info(),
+                rent_collector.to_account_info(),
+            )?;
+        }
 
-    /// Closes a `ConfigTransaction` and the corresponding `Proposal`.
-    /// `transaction` can be closed if either:
-    /// - the `proposal` is in a terminal state: `Executed`, `Rejected`, or `Cancelled`.
-    /// - the `proposal` is stale.
-    #[access_control(_ctx.accounts.validate())]
-    pub fn config_transaction_accounts_close(_ctx: Context<Self>) -> Result<()> {
-        // Anchor will close the accounts for us.
+        // Anchor will close the `transaction` account for us.
         Ok(())
     }
 }
@@ -105,14 +129,18 @@ pub struct VaultTransactionAccountsClose<'info> {
     )]
     pub multisig: Account<'info, Multisig>,
 
-    #[account(mut,
+    /// CHECK: `seeds` and `bump` verify that the account is the canonical Proposal,
+    ///         the logic within `vault_transaction_accounts_close` does the rest of the checks.
+    #[account(
+        mut,
         seeds = [
             SEED_PREFIX,
             multisig.key().as_ref(),
             SEED_TRANSACTION,
             &transaction.index.to_le_bytes(),
             SEED_PROPOSAL,
-        ], bump,
+        ],
+        bump,
     )]
     pub proposal: AccountInfo<'info>,
 
@@ -136,19 +164,30 @@ pub struct VaultTransactionAccountsClose<'info> {
 }
 
 impl VaultTransactionAccountsClose<'_> {
-    fn validate(&self) -> Result<()> {
-        let Self {
-            multisig, proposal, transaction, ..
-        } = self;
+    /// Closes a `VaultTransaction` and the corresponding `Proposal`.
+    /// `transaction` can be closed if either:
+    /// - the `proposal` is in a terminal state: `Executed`, `Rejected`, or `Cancelled`.
+    /// - the `proposal` is stale and not `Approved`.
+    pub fn vault_transaction_accounts_close(
+        ctx: Context<VaultTransactionAccountsClose>,
+    ) -> Result<()> {
+        let multisig = &ctx.accounts.multisig;
+        let transaction = &ctx.accounts.transaction;
+        let proposal = &mut ctx.accounts.proposal;
+        let rent_collector = &ctx.accounts.rent_collector;
 
         let is_stale = transaction.index <= multisig.stale_transaction_index;
 
-        let proposal_account: Option<Proposal> = if proposal.data.borrow().len() > 0 {
-            Some( Proposal::try_from_slice(&proposal.data.borrow())? )
-        } else { None };
+        let proposal_account = if proposal.data.borrow().is_empty() {
+            None
+        } else {
+            Some(Proposal::try_deserialize(
+                &mut &**proposal.data.borrow_mut(),
+            )?)
+        };
 
         #[allow(deprecated)]
-        let can_close = if let Some(proposal_account) = proposal_account {
+        let can_close = if let Some(proposal_account) = &proposal_account {
             match proposal_account.status {
                 // Draft proposals can only be closed if stale,
                 // so they can't be activated anymore.
@@ -169,21 +208,21 @@ impl VaultTransactionAccountsClose<'_> {
                 ProposalStatus::Executing => false,
             }
         } else {
+            // If no Proposal account exists then the VaultTransaction can only be closed if stale
             is_stale
         };
 
         require!(can_close, MultisigError::InvalidProposalStatus);
 
-        Ok(())
-    }
+        // Close the `proposal` account if exists.
+        if proposal_account.is_some() {
+            utils::close(
+                ctx.accounts.proposal.to_account_info(),
+                rent_collector.to_account_info(),
+            )?;
+        }
 
-    /// Closes a `VaultTransaction` and the corresponding `Proposal`.
-    /// `transaction` can be closed if either:
-    /// - the `proposal` is in a terminal state: `Executed`, `Rejected`, or `Cancelled`.
-    /// - the `proposal` is stale and not `Approved`.
-    #[access_control(_ctx.accounts.validate())]
-    pub fn vault_transaction_accounts_close(_ctx: Context<Self>) -> Result<()> {
-        // Anchor will close the accounts for us.
+        // Anchor will close the `transaction` account for us.
         Ok(())
     }
 }

programs/squads_multisig_program/src/utils/system.rs

@@ -99,3 +99,17 @@ pub fn create_account<'a, 'info>(
         )
     }
 }
+
+/// Closes an account by transferring all lamports to the `sol_destination`.
+///
+/// Lifted from private `anchor_lang::common::close`: https://github.com/coral-xyz/anchor/blob/714d5248636493a3d1db1481f16052836ee59e94/lang/src/common.rs#L6
+pub fn close<'info>(info: AccountInfo<'info>, sol_destination: AccountInfo<'info>) -> Result<()> {
+    // Transfer tokens from the account to the sol_destination.
+    let dest_starting_lamports = sol_destination.lamports();
+    **sol_destination.lamports.borrow_mut() =
+        dest_starting_lamports.checked_add(info.lamports()).unwrap();
+    **info.lamports.borrow_mut() = 0;
+
+    info.assign(&system_program::ID);
+    info.realloc(0, false).map_err(Into::into)
+}

sdk/multisig/idl/squads_multisig_program.json

@@ -1233,7 +1233,10 @@
         {
           "name": "proposal",
           "isMut": true,
-          "isSigner": false
+          "isSigner": false,
+          "docs": [
+            "the logic within `config_transaction_accounts_close` does the rest of the checks."
+          ]
         },
         {
           "name": "transaction",
@@ -1276,7 +1279,10 @@
         {
           "name": "proposal",
           "isMut": true,
-          "isSigner": false
+          "isSigner": false,
+          "docs": [
+            "the logic within `vault_transaction_accounts_close` does the rest of the checks."
+          ]
         },
         {
           "name": "transaction",

tests/suites/instructions/configTransactionAccountsClose.ts

@@ -25,11 +25,12 @@ describe("Instructions / config_transaction_accounts_close", () => {
   let members: TestMembers;
   let multisigPda: PublicKey;
   const staleTransactionIndex = 1n;
-  const executedTransactionIndex = 2n;
-  const activeTransactionIndex = 3n;
-  const approvedTransactionIndex = 4n;
-  const rejectedTransactionIndex = 5n;
-  const cancelledTransactionIndex = 6n;
+  const staleNoProposalTransactionIndex = 2n;
+  const executedTransactionIndex = 3n;
+  const activeTransactionIndex = 4n;
+  const approvedTransactionIndex = 5n;
+  const rejectedTransactionIndex = 6n;
+  const cancelledTransactionIndex = 7n;
 
   // Set up a multisig with config transactions.
   before(async () => {
@@ -82,6 +83,24 @@ describe("Instructions / config_transaction_accounts_close", () => {
     // This transaction will become stale when the second config transaction is executed.
     //endregion
 
+    //region Stale and No Proposal
+    // Create a config transaction (Stale and No Proposal).
+    signature = await multisig.rpc.configTransactionCreate({
+      connection,
+      feePayer: members.proposer,
+      multisigPda,
+      transactionIndex: staleNoProposalTransactionIndex,
+      creator: members.proposer.publicKey,
+      actions: [{ __kind: "ChangeThreshold", newThreshold: 1 }],
+      programId,
+    });
+    await connection.confirmTransaction(signature);
+
+    // No proposal created for this transaction.
+
+    // This transaction will become stale when the config transaction is executed.
+    //endregion
+
     //region Executed
     // Create a config transaction (Executed).
     signature = await multisig.rpc.configTransactionCreate({
@@ -524,7 +543,7 @@ describe("Instructions / config_transaction_accounts_close", () => {
         connection
           .sendTransaction(tx)
           .catch(multisig.errors.translateAndThrowAnchorError),
-      /Proposal is for another multisig/
+      /A seeds constraint was violated/
     );
   });
 
@@ -621,7 +640,7 @@ describe("Instructions / config_transaction_accounts_close", () => {
           rentCollector: vaultPda,
           proposal: multisig.getProposalPda({
             multisigPda,
-            transactionIndex: rejectedTransactionIndex,
+            transactionIndex: 1n,
             programId,
           })[0],
           transaction: multisig.getTransactionPda({
@@ -693,7 +712,7 @@ describe("Instructions / config_transaction_accounts_close", () => {
         connection
           .sendTransaction(tx)
           .catch(multisig.errors.translateAndThrowAnchorError),
-      /Transaction doesn't match proposal/
+      /A seeds constraint was violated/
     );
   });
 
@@ -744,6 +763,53 @@ describe("Instructions / config_transaction_accounts_close", () => {
     assert.ok(postBalance === preBalance + accountsRent);
   });
 
+  it("close accounts for Stale transaction with No Proposal", async () => {
+    const transactionIndex = staleNoProposalTransactionIndex;
+
+    const multisigAccount = await Multisig.fromAccountAddress(
+      connection,
+      multisigPda
+    );
+
+    // Make sure there's no proposal.
+    let proposalAccount = await connection.getAccountInfo(
+      multisig.getProposalPda({
+        multisigPda,
+        transactionIndex,
+        programId,
+      })[0]
+    );
+    assert.equal(proposalAccount, null);
+
+    // Make sure the transaction is stale.
+    assert.ok(
+      transactionIndex <=
+        multisig.utils.toBigInt(multisigAccount.staleTransactionIndex)
+    );
+
+    const [vaultPda] = multisig.getVaultPda({
+      multisigPda,
+      index: 0,
+      programId,
+    });
+
+    const preBalance = await connection.getBalance(vaultPda);
+
+    const sig = await multisig.rpc.configTransactionAccountsClose({
+      connection,
+      feePayer: members.almighty,
+      multisigPda,
+      rentCollector: vaultPda,
+      transactionIndex,
+      programId,
+    });
+    await connection.confirmTransaction(sig);
+
+    const postBalance = await connection.getBalance(vaultPda);
+    const accountsRent = 1_503_360; // Rent for the transaction account.
+    assert.equal(postBalance, preBalance + accountsRent);
+  });
+
   it("close accounts for Executed transaction", async () => {
     const transactionIndex = executedTransactionIndex;