remove duplicated multisig_signer in transaction execution

coral-xyz · Feb 10, 2022 · 9037d69 · 9037d69
1 parent a0e5d86
commit 9037d69
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 44 deletions.
diff --git a/programs/multisig/src/lib.rs b/programs/multisig/src/lib.rs
@@ -59,6 +59,24 @@ pub mod serum_multisig {
         accs: Vec<TransactionAccount>,
         data: Vec<u8>,
     ) -> Result<()> {
+        let multisig_signer = Pubkey::create_program_address(
+            &[
+                ctx.accounts.multisig.key().as_ref(),
+                &[ctx.accounts.multisig.nonce],
+            ],
+            ctx.program_id,
+        )
+        .expect("valid nonce");
+        if let Some(account) = accs
+            .iter()
+            .find(|account| account.pubkey == multisig_signer)
+        {
+            require!(
+                account.is_signer && !account.is_writable,
+                InvalidMultisigSigner
+            );
+        }
+
         let owner_index = ctx
             .accounts
             .multisig
@@ -167,18 +185,7 @@ pub mod serum_multisig {
         }
 
         // Execute the transaction signed by the multisig.
-        let mut ix: Instruction = (*ctx.accounts.transaction).deref().into();
-        ix.accounts = ix
-            .accounts
-            .iter()
-            .map(|acc| {
-                let mut acc = acc.clone();
-                if &acc.pubkey == ctx.accounts.multisig_signer.key {
-                    acc.is_signer = true;
-                }
-                acc
-            })
-            .collect();
+        let ix: Instruction = (*ctx.accounts.transaction).deref().into();
         let multisig_key = ctx.accounts.multisig.key();
         let seeds = &[multisig_key.as_ref(), &[ctx.accounts.multisig.nonce]];
         let signer = &[&seeds[..]];
@@ -232,11 +239,6 @@ pub struct Auth<'info> {
 pub struct ExecuteTransaction<'info> {
     #[account(constraint = multisig.owner_set_seqno == transaction.owner_set_seqno)]
     multisig: Box<Account<'info, Multisig>>,
-    #[account(
-        seeds = [multisig.key().as_ref()],
-        bump = multisig.nonce,
-    )]
-    multisig_signer: UncheckedAccount<'info>,
     #[account(mut, has_one = multisig)]
     transaction: Box<Account<'info, Transaction>>,
 }
@@ -333,4 +335,6 @@ pub enum ErrorCode {
     InvalidThreshold,
     #[msg("Owners must be unique")]
     UniqueOwners,
+    #[msg("Invalid MultisigSigner in transaction accounts.")]
+    InvalidMultisigSigner,
 }
diff --git a/tests/multisig.js b/tests/multisig.js
@@ -44,27 +44,17 @@ describe("multisig", () => {
     assert.deepStrictEqual(multisigAccount.owners, owners);
     assert.ok(multisigAccount.ownerSetSeqno === 0);
 
-    const pid = program.programId;
-    const accounts = [
-      {
-        pubkey: multisig.publicKey,
-        isWritable: true,
-        isSigner: false,
-      },
-      {
-        pubkey: multisigSigner,
-        isWritable: false,
-        isSigner: true,
-      },
-    ];
     const newOwners = [ownerA.publicKey, ownerB.publicKey, ownerD.publicKey];
-    const data = program.coder.instruction.encode("set_owners", {
-      owners: newOwners,
+    const ix = program.instruction.setOwners(newOwners, {
+      accounts: {
+        multisig: multisig.publicKey,
+        multisigSigner,
+      }
     });
 
     const transaction = anchor.web3.Keypair.generate();
     const txSize = 1000; // Big enough, cuz I'm lazy.
-    await program.rpc.createTransaction(pid, accounts, data, {
+    await program.rpc.createTransaction(ix.programId, ix.keys, ix.data, {
       accounts: {
         multisig: multisig.publicKey,
         transaction: transaction.publicKey,
@@ -83,9 +73,9 @@ describe("multisig", () => {
       transaction.publicKey
     );
 
-    assert.ok(txAccount.programId.equals(pid));
-    assert.deepStrictEqual(txAccount.accounts, accounts);
-    assert.deepStrictEqual(txAccount.data, data);
+    assert.ok(txAccount.programId.equals(ix.programId));
+    assert.deepStrictEqual(txAccount.accounts, ix.keys);
+    assert.deepStrictEqual(txAccount.data, ix.data);
     assert.ok(txAccount.multisig.equals(multisig.publicKey));
     assert.deepStrictEqual(txAccount.didExecute, false);
     assert.ok(txAccount.ownerSetSeqno === 0);
@@ -100,26 +90,20 @@ describe("multisig", () => {
       signers: [ownerB],
     });
 
-    // Now that we've reached the threshold, send the transactoin.
     await program.rpc.executeTransaction({
       accounts: {
         multisig: multisig.publicKey,
-        multisigSigner,
         transaction: transaction.publicKey,
       },
-      remainingAccounts: program.instruction.setOwners
-        .accounts({
-          multisig: multisig.publicKey,
-          multisigSigner,
-        })
+      remainingAccounts: ix.keys
         // Change the signer status on the vendor signer since it's signed by the program, not the client.
         .map((meta) =>
           meta.pubkey.equals(multisigSigner)
             ? { ...meta, isSigner: false }
             : meta
         )
         .concat({
-          pubkey: program.programId,
+          pubkey: ix.programId,
           isWritable: false,
           isSigner: false,
         }),