Merge pull request #128 from Squads-Protocol/osec-fixes

General Fixes to Transaction Buffer Logic

programs/squads_multisig_program/src/allocator.rs

@@ -103,27 +103,22 @@ struct BumpAllocator;
 unsafe impl std::alloc::GlobalAlloc for BumpAllocator {
     #[inline]
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-        if layout.size() == isize::MAX as usize - 0x42 {
-            // Return test value
-            0x42 as *mut u8
-        } else {
-            const POS_PTR: *mut usize = HEAP_START_ADDRESS as *mut usize;
-            const TOP_ADDRESS: usize = HEAP_START_ADDRESS as usize + HEAP_LENGTH;
-            const BOTTOM_ADDRESS: usize = HEAP_START_ADDRESS as usize + size_of::<*mut u8>();
-            let mut pos = *POS_PTR;
-            if pos == 0 {
-                // First time, set starting position to bottom address
-                pos = BOTTOM_ADDRESS;
-            }
-            // Align the position upwards
-            pos = (pos + layout.align() - 1) & !(layout.align() - 1);
-            let next_pos = pos.saturating_add(layout.size());
-            if next_pos > TOP_ADDRESS {
-                return null_mut();
-            }
-            *POS_PTR = next_pos;
-            pos as *mut u8
+        const POS_PTR: *mut usize = HEAP_START_ADDRESS as *mut usize;
+        const TOP_ADDRESS: usize = HEAP_START_ADDRESS as usize + HEAP_LENGTH;
+        const BOTTOM_ADDRESS: usize = HEAP_START_ADDRESS as usize + size_of::<*mut u8>();
+        let mut pos = *POS_PTR;
+        if pos == 0 {
+            // First time, set starting position to bottom address
+            pos = BOTTOM_ADDRESS;
         }
+        // Align the position upwards
+        pos = (pos + layout.align() - 1) & !(layout.align() - 1);
+        let next_pos = pos.saturating_add(layout.size());
+        if next_pos > TOP_ADDRESS {
+            return null_mut();
+        }
+        *POS_PTR = next_pos;
+        pos as *mut u8
     }
 
     #[inline]

programs/squads_multisig_program/src/instructions/mod.rs

@@ -3,7 +3,6 @@ pub use batch_create::*;
 pub use batch_execute_transaction::*;
 pub use config_transaction_create::*;
 pub use config_transaction_execute::*;
-pub use heap_test::*;
 pub use multisig_add_spending_limit::*;
 pub use multisig_config::*;
 pub use multisig_create::*;
@@ -27,7 +26,6 @@ mod batch_create;
 mod batch_execute_transaction;
 mod config_transaction_create;
 mod config_transaction_execute;
-mod heap_test;
 mod multisig_add_spending_limit;
 mod multisig_config;
 mod multisig_create;

programs/squads_multisig_program/src/instructions/transaction_buffer_close.rs

@@ -6,7 +6,6 @@ use crate::state::*;
 #[derive(Accounts)]
 pub struct TransactionBufferClose<'info> {
     #[account(
-        mut,
         seeds = [SEED_PREFIX, SEED_MULTISIG, multisig.create_key.as_ref()],
         bump = multisig.bump,
     )]
@@ -24,6 +23,7 @@ pub struct TransactionBufferClose<'info> {
             SEED_PREFIX,
             multisig.key().as_ref(),
             SEED_TRANSACTION_BUFFER,
+            creator.key().as_ref(),
             &transaction_buffer.buffer_index.to_le_bytes()
         ],
         bump
@@ -36,16 +36,6 @@ pub struct TransactionBufferClose<'info> {
 
 impl TransactionBufferClose<'_> {
     fn validate(&self) -> Result<()> {
-        let Self {
-            multisig, creator, ..
-        } = self;
-
-        // creator is still a member in the multisig
-        require!(
-            multisig.is_member(creator.key()).is_some(),
-            MultisigError::NotAMember
-        );
-
         Ok(())
     }
 

programs/squads_multisig_program/src/instructions/transaction_buffer_create.rs

@@ -22,7 +22,6 @@ pub struct TransactionBufferCreateArgs {
 #[instruction(args: TransactionBufferCreateArgs)]
 pub struct TransactionBufferCreate<'info> {
     #[account(
-        mut,
         seeds = [SEED_PREFIX, SEED_MULTISIG, multisig.create_key.as_ref()],
         bump = multisig.bump,
     )]
@@ -36,6 +35,7 @@ pub struct TransactionBufferCreate<'info> {
             SEED_PREFIX,
             multisig.key().as_ref(),
             SEED_TRANSACTION_BUFFER,
+            creator.key().as_ref(),
             &args.buffer_index.to_le_bytes(),
         ],
         bump

programs/squads_multisig_program/src/instructions/transaction_buffer_extend.rs

@@ -13,7 +13,6 @@ pub struct TransactionBufferExtendArgs {
 #[instruction(args: TransactionBufferExtendArgs)]
 pub struct TransactionBufferExtend<'info> {
     #[account(
-        mut,
         seeds = [SEED_PREFIX, SEED_MULTISIG, multisig.create_key.as_ref()],
         bump = multisig.bump,
     )]
@@ -27,6 +26,7 @@ pub struct TransactionBufferExtend<'info> {
             SEED_PREFIX,
             multisig.key().as_ref(),
             SEED_TRANSACTION_BUFFER,
+            creator.key().as_ref(),
             &transaction_buffer.buffer_index.to_le_bytes()
         ],
         bump

programs/squads_multisig_program/src/instructions/vault_transaction_create_from_buffer.rs

@@ -11,10 +11,14 @@ pub struct VaultTransactionCreateFromBuffer<'info> {
     #[account(
         mut,
         close = creator,
+        // Only the creator can turn the buffer into a transaction and reclaim
+        // the rent
+        constraint = transaction_buffer.creator == creator.key() @ MultisigError::Unauthorized,
         seeds = [
             SEED_PREFIX,
             vault_transaction_create.multisig.key().as_ref(),
             SEED_TRANSACTION_BUFFER,
+            creator.key().as_ref(),
             &transaction_buffer.buffer_index.to_le_bytes(),
         ],
         bump

programs/squads_multisig_program/src/lib.rs

@@ -265,7 +265,10 @@ pub mod squads_multisig_program {
     /// accommodate the new amount of cancel votes.
     /// The previous implemenation still works if the proposal size is in line with the
     /// threshold size.
-    pub fn proposal_cancel_v2<'info>(ctx: Context<'_, '_, 'info, 'info, ProposalCancelV2<'info>>, args: ProposalVoteArgs) -> Result<()> {
+    pub fn proposal_cancel_v2<'info>(
+        ctx: Context<'_, '_, 'info, 'info, ProposalCancelV2<'info>>,
+        args: ProposalVoteArgs,
+    ) -> Result<()> {
         ProposalCancelV2::proposal_cancel_v2(ctx, args)
     }
 
@@ -316,8 +319,4 @@ pub mod squads_multisig_program {
     pub fn batch_accounts_close(ctx: Context<BatchAccountsClose>) -> Result<()> {
         BatchAccountsClose::batch_accounts_close(ctx)
     }
-    // Uncomment to enable the heap_test instruction
-    // pub fn heap_test(ctx: Context<HeapTest>, length: u64) -> Result<()> {
-    //     HeapTest::handler(ctx, length)
-    // }
 }

programs/squads_multisig_program/src/state/transaction_buffer.rs

@@ -66,7 +66,7 @@ impl TransactionBuffer {
             MultisigError::FinalBufferSizeExceeded
         );
         require!(
-            self.buffer.len() < MAX_BUFFER_SIZE,
+            self.buffer.len() <= MAX_BUFFER_SIZE,
             MultisigError::FinalBufferSizeExceeded
         );
         require!(

sdk/multisig/idl/squads_multisig_program.json

@@ -763,7 +763,7 @@
       "accounts": [
         {
           "name": "multisig",
-          "isMut": true,
+          "isMut": false,
           "isSigner": false
         },
         {
@@ -810,7 +810,7 @@
       "accounts": [
         {
           "name": "multisig",
-          "isMut": true,
+          "isMut": false,
           "isSigner": false
         },
         {
@@ -837,7 +837,7 @@
       "accounts": [
         {
           "name": "multisig",
-          "isMut": true,
+          "isMut": false,
           "isSigner": false
         },
         {

tests/suites/examples/transaction-buffer.ts

@@ -103,6 +103,7 @@ describe("Examples / Transaction Buffers", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.almighty.publicKey.toBuffer(),
         Buffer.from([bufferIndex])
       ],
       programId

tests/suites/instructions/transactionBufferClose.ts

@@ -83,6 +83,7 @@ describe("Instructions / transaction_buffer_close", () => {
                 Buffer.from("multisig"),
                 multisigPda.toBuffer(),
                 Buffer.from("transaction_buffer"),
+                members.proposer.publicKey.toBuffer(),
                 Uint8Array.from([bufferIndex])
             ],
             programId
@@ -149,7 +150,7 @@ describe("Instructions / transaction_buffer_close", () => {
                 connection
                     .sendTransaction(closeTx)
                     .catch(multisig.errors.translateAndThrowAnchorError),
-            /Unauthorized/
+            /(Unauthorized|ConstraintSeeds)/
         );
     });
 

tests/suites/instructions/transactionBufferCreate.ts

@@ -96,6 +96,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.proposer.publicKey.toBuffer(),
         Uint8Array.from([bufferIndex])
       ],
       programId
@@ -165,6 +166,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.proposer.publicKey.toBuffer(),
         Uint8Array.from([bufferIndex])
       ],
       programId
@@ -233,6 +235,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.proposer.publicKey.toBuffer(),
         Uint8Array.from([bufferIndex])
       ],
       programId
@@ -365,6 +368,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        nonMember.publicKey.toBuffer(),
         Uint8Array.from([bufferIndex]),
       ],
       programId
@@ -452,6 +456,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        memberWithoutInitiatePermissions.publicKey.toBuffer(),
         Uint8Array.from([bufferIndex]),
       ],
       programId
@@ -538,6 +543,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.proposer.publicKey.toBuffer(),
         Buffer.from(invalidBufferIndex),
       ],
       programId
@@ -604,6 +610,7 @@ describe("Instructions / transaction_buffer_create", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.proposer.publicKey.toBuffer(),
         Uint8Array.from([bufferIndex]),
       ],
       programId

tests/suites/instructions/transactionBufferExtend.ts

@@ -74,6 +74,7 @@ describe("Instructions / transaction_buffer_extend", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        creator.publicKey.toBuffer(),
         Buffer.from([Number(transactionIndex)])
       ],
       programId
@@ -194,6 +195,7 @@ describe("Instructions / transaction_buffer_extend", () => {
         Buffer.from("multisig"),
         multisigPda.toBuffer(),
         Buffer.from("transaction_buffer"),
+        members.proposer.publicKey.toBuffer(),
         Buffer.from([Number(transactionIndex)])
       ],
       programId
@@ -353,7 +355,7 @@ describe("Instructions / transaction_buffer_extend", () => {
 
     await assert.rejects(
       () => connection.sendTransaction(tx).catch(multisig.errors.translateAndThrowAnchorError),
-      /Unauthorized/
+      /(Unauthorized|ConstraintSeeds)/
     );
 
     await closeTransactionBuffer(members.almighty, transactionBuffer);
@@ -429,7 +431,7 @@ describe("Instructions / transaction_buffer_extend", () => {
 
     await assert.rejects(
       () => connection.sendTransaction(extendTx).catch(multisig.errors.translateAndThrowAnchorError),
-      /Unauthorized/
+      /(Unauthorized|ConstraintSeeds)/
     );