s/threhsold/threshold/ (#65)

for community reuse

.gitignore

@@ -1,2 +1,3 @@
 .anchor
-target
+node_modules
+target

.travis.yml

@@ -5,24 +5,39 @@ rust:
 env:
   global:
     - NODE_VERSION="v14.7.0"
-    - SOLANA_VERSION="v1.6.6"
-    - ANCHOR_VERSION="v0.4.4"
+    - SOLANA_VERSION="v1.10.29"
+    - ANCHOR_VERSION="v0.25.0"
+
+cache:
+  directories:
+    # solana
+    - $HOME/.cache/solana/
+    - $HOME/.local/share/solana/
+    # Cargo
+    - $HOME/.cargo/bin/
+    - $HOME/.cargo/registry/index/
+    - $HOME/.cargo/registry/cache/
+    - $HOME/.cargo/git/db/
+    - $TRAVIS_BUILD_DIR/target
+    # npm
+    - $HOME/.npm
+    # - $TRAVIS_BUILD_DIR/node_modules    
 
 before_deploy:
   - anchor build --verifiable
   - echo "### SHA256 Checksums" > release_notes.md
-  - sha256sum target/deploy/multisig.so > binary.txt
-  - sha256sum target/idl/multisig.json > idl.txt
+  - sha256sum target/deploy/coral_multisig.so > binary.txt
+  - sha256sum target/idl/coral_multisig.json > idl.txt
   - cat *.txt >> release_notes.md
   - echo "" >> release_notes.md
-  - echo "Built with Anchor [${ANCHOR_VERSION}](https://github.com/project-serum/anchor/releases/tag/${ANCHOR_VERSION})." >> release_notes.md
+  - echo "Built with Anchor [${ANCHOR_VERSION}](https://github.com/coral-xyz/anchor/releases/tag/${ANCHOR_VERSION})." >> release_notes.md
 
 deploy:
   provider: releases
   edge: true
   file:
-    - "target/deploy/multisig.so"
-    - "target/idl/multisig.json"
+    - "target/deploy/coral_multisig.so"
+    - "target/idl/coral_multisig.json"
   release_notes_file: release_notes.md
   skip_cleanup: true
   on:
@@ -33,18 +48,24 @@ deploy:
 _defaults: &defaults
   before_install:
   - nvm install $NODE_VERSION
-  - npm install -g mocha
-  - npm install -g @project-serum/anchor
+  - npm ci
   - sudo apt-get install -y pkg-config build-essential libudev-dev
   - sh -c "$(curl -sSfL https://release.solana.com/${SOLANA_VERSION}/install)"
   - export PATH="/home/travis/.local/share/solana/install/active_release/bin:$PATH"
   - export NODE_PATH="/home/travis/.nvm/versions/node/${NODE_VERSION}/lib/node_modules/:${NODE_PATH}"
   - yes | solana-keygen new
-  - cargo install --git https://github.com/project-serum/anchor --tag ${ANCHOR_VERSION} anchor-cli --locked
 
 jobs:
   include:
     - <<: *defaults
       name: Runs the tests
       script:
-        - anchor test
+        - npx anchor build
+        - npx anchor test
+    - <<: *defaults
+      name: Linting & clippy
+      script:
+        - npm run lint
+        - rustup component add rustfmt clippy
+        - cargo-fmt --all -- --check
+        - cargo clippy --all-targets

Anchor.toml

@@ -1,2 +1,21 @@
+[features]
+seeds = true
+
+[programs.mainnet]
+coral_multisig = "msigUdDBsR4zSUYqYEDrc1LcgtmuSDDM7KxpRUXNC6U"
+
+[programs.devnet]
+coral_multisig = "msigUdDBsR4zSUYqYEDrc1LcgtmuSDDM7KxpRUXNC6U"
+
+[programs.localnet]
+coral_multisig = "msigUdDBsR4zSUYqYEDrc1LcgtmuSDDM7KxpRUXNC6U"
+
+[registry]
+url = "https://api.apr.dev"
+
+[provider]
 cluster = "localnet"
 wallet = "~/.config/solana/id.json"
+
+[scripts]
+test = "npx mocha -t 1000000 tests/"

Cargo.toml

@@ -2,3 +2,13 @@
 members = [
     "programs/*"
 ]
+
+[profile.release]
+overflow-checks = true
+lto = "fat"
+codegen-units = 1
+
+[profile.release.build-override]
+opt-level = 3
+incremental = false
+codegen-units = 1

README.md

@@ -16,17 +16,17 @@ Once the `Multisig` account is created, one can create a `Transaction`
 account, specifying the parameters for a normal solana transaction.
 
 To sign, owners should invoke the `approve` instruction, and finally,
-the `execute_transaction`, once enough (i.e. `threhsold`) of the owners have
+the `execute_transaction`, once enough (i.e. `threshold`) of the owners have
 signed.
 
 ## Note
 
-* **This code is unaudited. Use at your own risk.**
+* **This code (0.9.0) is [audited](./SECURITY_AUDIT_REPORT.pdf). Audit commissioned by [Streamflow](https://github.com/streamflow-finance).**
 
 ## Developing
 
-[Anchor](https://github.com/project-serum/anchor) is used for developoment, and it's
-recommended workflow is used here. To get started, see the [guide](https://project-serum.github.io/anchor/getting-started/introduction.html).
+[Anchor](https://github.com/coral-xyz/anchor) is used for developoment, and it's
+recommended workflow is used here. To get started, see the [guide](https://anchor-lang.com).
 
 ### Build
 
@@ -52,4 +52,4 @@ docker, `cd programs/multisig`, and run
 anchor verify <program-id | write-buffer>
 ```
 
-A list of build artifacts can be found under [releases](https://github.com/project-serum/multisig/releases).
+A list of build artifacts can be found under [releases](https://github.com/coral-xyz/multisig/releases).

package.json

@@ -0,0 +1,14 @@
+{
+  "private": true,
+  "scripts": {
+    "lint": "npx prettier --check tests/",
+    "lint:fix": "npx prettier --write tests/",
+    "test": "npx anchor test"
+  },
+  "devDependencies": {
+    "@project-serum/anchor": "=0.24.2",
+    "@project-serum/anchor-cli": "=0.24.2",
+    "mocha": "^9.1.3",
+    "prettier": "^2.5.1"
+  }
+}

programs/multisig/Cargo.toml

@@ -1,12 +1,13 @@
 [package]
-name = "multisig"
-version = "0.4.0"
+name = "coral-multisig"
+version = "0.9.0"
 description = "Created with Anchor"
-edition = "2018"
+edition = "2021"
+license = "Apache-2.0"
 
 [lib]
 crate-type = ["cdylib", "lib"]
-name = "multisig"
+name = "coral_multisig"
 
 [features]
 no-entrypoint = []
@@ -15,4 +16,4 @@ cpi = ["no-entrypoint"]
 default = []
 
 [dependencies]
-anchor-lang = "0.4.4"
+anchor-lang = "0.25.0"

programs/multisig/src/lib.rs

@@ -14,16 +14,19 @@
 //! account, specifying the parameters for a normal solana transaction.
 //!
 //! To sign, owners should invoke the `approve` instruction, and finally,
-//! the `execute_transaction`, once enough (i.e. `threhsold`) of the owners have
+//! the `execute_transaction`, once enough (i.e. `threshold`) of the owners have
 //! signed.
 
 use anchor_lang::prelude::*;
 use anchor_lang::solana_program;
 use anchor_lang::solana_program::instruction::Instruction;
 use std::convert::Into;
+use std::ops::Deref;
+
+declare_id!("msigUdDBsR4zSUYqYEDrc1LcgtmuSDDM7KxpRUXNC6U");
 
 #[program]
-pub mod multisig {
+pub mod coral_multisig {
     use super::*;
 
     // Initializes a new multisig account with a set of owners and a threshold.
@@ -33,6 +36,13 @@ pub mod multisig {
         threshold: u64,
         nonce: u8,
     ) -> Result<()> {
+        assert_unique_owners(&owners)?;
+        require!(
+            threshold > 0 && threshold <= owners.len() as u64,
+            InvalidThreshold
+        );
+        require!(!owners.is_empty(), InvalidOwnersLen);
+
         let multisig = &mut ctx.accounts.multisig;
         multisig.owners = owners;
         multisig.threshold = threshold;
@@ -66,7 +76,7 @@ pub mod multisig {
         tx.accounts = accs;
         tx.data = data;
         tx.signers = signers;
-        tx.multisig = *ctx.accounts.multisig.to_account_info().key;
+        tx.multisig = ctx.accounts.multisig.key();
         tx.did_execute = false;
         tx.owner_set_seqno = ctx.accounts.multisig.owner_set_seqno;
 
@@ -88,9 +98,30 @@ pub mod multisig {
         Ok(())
     }
 
+    // Set owners and threshold at once.
+    pub fn set_owners_and_change_threshold<'info>(
+        ctx: Context<'_, '_, '_, 'info, Auth<'info>>,
+        owners: Vec<Pubkey>,
+        threshold: u64,
+    ) -> Result<()> {
+        set_owners(
+            Context::new(
+                ctx.program_id,
+                ctx.accounts,
+                ctx.remaining_accounts,
+                ctx.bumps.clone(),
+            ),
+            owners,
+        )?;
+        change_threshold(ctx, threshold)
+    }
+
     // Sets the owners field on the multisig. The only way this can be invoked
     // is via a recursive call from execute_transaction -> set_owners.
     pub fn set_owners(ctx: Context<Auth>, owners: Vec<Pubkey>) -> Result<()> {
+        assert_unique_owners(&owners)?;
+        require!(!owners.is_empty(), InvalidOwnersLen);
+
         let multisig = &mut ctx.accounts.multisig;
 
         if (owners.len() as u64) < multisig.threshold {
@@ -107,6 +138,7 @@ pub mod multisig {
     // invoked is via a recursive call from execute_transaction ->
     // change_threshold.
     pub fn change_threshold(ctx: Context<Auth>, threshold: u64) -> Result<()> {
+        require!(threshold > 0, InvalidThreshold);
         if threshold > ctx.accounts.multisig.owners.len() as u64 {
             return Err(ErrorCode::InvalidThreshold.into());
         }
@@ -128,36 +160,30 @@ pub mod multisig {
             .transaction
             .signers
             .iter()
-            .filter_map(|s| match s {
-                false => None,
-                true => Some(true),
-            })
-            .collect::<Vec<_>>()
-            .len() as u64;
+            .filter(|&did_sign| *did_sign)
+            .count() as u64;
         if sig_count < ctx.accounts.multisig.threshold {
             return Err(ErrorCode::NotEnoughSigners.into());
         }
 
         // Execute the transaction signed by the multisig.
-        let mut ix: Instruction = (&*ctx.accounts.transaction).into();
+        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 {
-                    AccountMeta::new_readonly(acc.pubkey, true)
-                } else {
-                    acc.clone()
+                    acc.is_signer = true;
                 }
+                acc
             })
             .collect();
-        let seeds = &[
-            ctx.accounts.multisig.to_account_info().key.as_ref(),
-            &[ctx.accounts.multisig.nonce],
-        ];
+        let multisig_key = ctx.accounts.multisig.key();
+        let seeds = &[multisig_key.as_ref(), &[ctx.accounts.multisig.nonce]];
         let signer = &[&seeds[..]];
         let accounts = ctx.remaining_accounts;
-        solana_program::program::invoke_signed(&ix, &accounts, signer)?;
+        solana_program::program::invoke_signed(&ix, accounts, signer)?;
 
         // Burn the transaction to ensure one time use.
         ctx.accounts.transaction.did_execute = true;
@@ -168,88 +194,85 @@ pub mod multisig {
 
 #[derive(Accounts)]
 pub struct CreateMultisig<'info> {
-    #[account(init)]
-    multisig: ProgramAccount<'info, Multisig>,
-    rent: Sysvar<'info, Rent>,
+    #[account(zero, signer)]
+    multisig: Box<Account<'info, Multisig>>,
 }
 
 #[derive(Accounts)]
 pub struct CreateTransaction<'info> {
-    multisig: ProgramAccount<'info, Multisig>,
-    #[account(init)]
-    transaction: ProgramAccount<'info, Transaction>,
+    multisig: Box<Account<'info, Multisig>>,
+    #[account(zero, signer)]
+    transaction: Box<Account<'info, Transaction>>,
     // One of the owners. Checked in the handler.
-    #[account(signer)]
-    proposer: AccountInfo<'info>,
-    rent: Sysvar<'info, Rent>,
+    proposer: Signer<'info>,
 }
 
 #[derive(Accounts)]
 pub struct Approve<'info> {
-    #[account("multisig.owner_set_seqno == transaction.owner_set_seqno")]
-    multisig: ProgramAccount<'info, Multisig>,
-    #[account(mut, belongs_to = multisig)]
-    transaction: ProgramAccount<'info, Transaction>,
+    #[account(constraint = multisig.owner_set_seqno == transaction.owner_set_seqno)]
+    multisig: Box<Account<'info, Multisig>>,
+    #[account(mut, has_one = multisig)]
+    transaction: Box<Account<'info, Transaction>>,
     // One of the multisig owners. Checked in the handler.
-    #[account(signer)]
-    owner: AccountInfo<'info>,
+    owner: Signer<'info>,
 }
 
 #[derive(Accounts)]
 pub struct Auth<'info> {
     #[account(mut)]
-    multisig: ProgramAccount<'info, Multisig>,
-    #[account(signer, seeds = [
-        multisig.to_account_info().key.as_ref(),
-        &[multisig.nonce],
-    ])]
-    multisig_signer: AccountInfo<'info>,
+    multisig: Box<Account<'info, Multisig>>,
+    #[account(
+        seeds = [multisig.key().as_ref()],
+        bump = multisig.nonce,
+    )]
+    multisig_signer: Signer<'info>,
 }
 
 #[derive(Accounts)]
 pub struct ExecuteTransaction<'info> {
-    #[account("multisig.owner_set_seqno == transaction.owner_set_seqno")]
-    multisig: ProgramAccount<'info, Multisig>,
-    #[account(seeds = [
-        multisig.to_account_info().key.as_ref(),
-        &[multisig.nonce],
-    ])]
-    multisig_signer: AccountInfo<'info>,
-    #[account(mut, belongs_to = multisig)]
-    transaction: ProgramAccount<'info, Transaction>,
+    #[account(constraint = multisig.owner_set_seqno == transaction.owner_set_seqno)]
+    multisig: Box<Account<'info, Multisig>>,
+    /// CHECK: multisig_signer is a PDA program signer. Data is never read or written to
+    #[account(
+        seeds = [multisig.key().as_ref()],
+        bump = multisig.nonce,
+    )]
+    multisig_signer: UncheckedAccount<'info>,
+    #[account(mut, has_one = multisig)]
+    transaction: Box<Account<'info, Transaction>>,
 }
 
 #[account]
 pub struct Multisig {
-    owners: Vec<Pubkey>,
-    threshold: u64,
-    nonce: u8,
-    owner_set_seqno: u32,
+    pub owners: Vec<Pubkey>,
+    pub threshold: u64,
+    pub nonce: u8,
+    pub owner_set_seqno: u32,
 }
 
 #[account]
 pub struct Transaction {
     // The multisig account this transaction belongs to.
-    multisig: Pubkey,
+    pub multisig: Pubkey,
     // Target program to execute against.
-    program_id: Pubkey,
+    pub program_id: Pubkey,
     // Accounts requried for the transaction.
-    accounts: Vec<TransactionAccount>,
+    pub accounts: Vec<TransactionAccount>,
     // Instruction data for the transaction.
-    data: Vec<u8>,
+    pub data: Vec<u8>,
     // signers[index] is true iff multisig.owners[index] signed the transaction.
-    signers: Vec<bool>,
+    pub signers: Vec<bool>,
     // Boolean ensuring one time execution.
-    did_execute: bool,
+    pub did_execute: bool,
     // Owner set sequence number.
-    owner_set_seqno: u32,
+    pub owner_set_seqno: u32,
 }
 
 impl From<&Transaction> for Instruction {
     fn from(tx: &Transaction) -> Instruction {
         Instruction {
             program_id: tx.program_id,
-            accounts: tx.accounts.clone().into_iter().map(Into::into).collect(),
+            accounts: tx.accounts.iter().map(Into::into).collect(),
             data: tx.data.clone(),
         }
     }
@@ -257,13 +280,13 @@ impl From<&Transaction> for Instruction {
 
 #[derive(AnchorSerialize, AnchorDeserialize, Clone)]
 pub struct TransactionAccount {
-    pubkey: Pubkey,
-    is_signer: bool,
-    is_writable: bool,
+    pub pubkey: Pubkey,
+    pub is_signer: bool,
+    pub is_writable: bool,
 }
 
-impl From<TransactionAccount> for AccountMeta {
-    fn from(account: TransactionAccount) -> AccountMeta {
+impl From<&TransactionAccount> for AccountMeta {
+    fn from(account: &TransactionAccount) -> AccountMeta {
         match account.is_writable {
             false => AccountMeta::new_readonly(account.pubkey, account.is_signer),
             true => AccountMeta::new(account.pubkey, account.is_signer),
@@ -271,10 +294,32 @@ impl From<TransactionAccount> for AccountMeta {
     }
 }
 
-#[error]
+impl From<&AccountMeta> for TransactionAccount {
+    fn from(account_meta: &AccountMeta) -> TransactionAccount {
+        TransactionAccount {
+            pubkey: account_meta.pubkey,
+            is_signer: account_meta.is_signer,
+            is_writable: account_meta.is_writable,
+        }
+    }
+}
+
+fn assert_unique_owners(owners: &[Pubkey]) -> Result<()> {
+    for (i, owner) in owners.iter().enumerate() {
+        require!(
+            !owners.iter().skip(i + 1).any(|item| item == owner),
+            UniqueOwners
+        )
+    }
+    Ok(())
+}
+
+#[error_code]
 pub enum ErrorCode {
     #[msg("The given owner is not part of this multisig.")]
     InvalidOwner,
+    #[msg("Owners length must be non zero.")]
+    InvalidOwnersLen,
     #[msg("Not enough owners signed this transaction.")]
     NotEnoughSigners,
     #[msg("Cannot delete a transaction that has been signed by an owner.")]
@@ -287,4 +332,6 @@ pub enum ErrorCode {
     AlreadyExecuted,
     #[msg("Threshold must be less than or equal to the number of owners.")]
     InvalidThreshold,
+    #[msg("Owners must be unique")]
+    UniqueOwners,
 }

tests/multisig.js

@@ -3,32 +3,29 @@ const assert = require("assert");
 
 describe("multisig", () => {
   // Configure the client to use the local cluster.
-  anchor.setProvider(anchor.Provider.env());
+  anchor.setProvider(anchor.getProvider());
 
-  const program = anchor.workspace.Multisig;
+  const program = anchor.workspace.CoralMultisig;
 
   it("Tests the multisig program", async () => {
-    const multisig = new anchor.web3.Account();
-    const [
-      multisigSigner,
-      nonce,
-    ] = await anchor.web3.PublicKey.findProgramAddress(
-      [multisig.publicKey.toBuffer()],
-      program.programId
-    );
+    const multisig = anchor.web3.Keypair.generate();
+    const [multisigSigner, nonce] =
+      await anchor.web3.PublicKey.findProgramAddress(
+        [multisig.publicKey.toBuffer()],
+        program.programId
+      );
     const multisigSize = 200; // Big enough.
 
-    const ownerA = new anchor.web3.Account();
-    const ownerB = new anchor.web3.Account();
-    const ownerC = new anchor.web3.Account();
-    const ownerD = new anchor.web3.Account();
+    const ownerA = anchor.web3.Keypair.generate();
+    const ownerB = anchor.web3.Keypair.generate();
+    const ownerC = anchor.web3.Keypair.generate();
+    const ownerD = anchor.web3.Keypair.generate();
     const owners = [ownerA.publicKey, ownerB.publicKey, ownerC.publicKey];
 
     const threshold = new anchor.BN(2);
     await program.rpc.createMultisig(owners, threshold, nonce, {
       accounts: {
         multisig: multisig.publicKey,
-        rent: anchor.web3.SYSVAR_RENT_PUBKEY,
       },
       instructions: [
         await program.account.multisig.createInstruction(
@@ -39,10 +36,12 @@ describe("multisig", () => {
       signers: [multisig],
     });
 
-    let multisigAccount = await program.account.multisig(multisig.publicKey);
-    assert.equal(multisigAccount.nonce, nonce);
+    let multisigAccount = await program.account.multisig.fetch(
+      multisig.publicKey
+    );
+    assert.strictEqual(multisigAccount.nonce, nonce);
     assert.ok(multisigAccount.threshold.eq(new anchor.BN(2)));
-    assert.deepEqual(multisigAccount.owners, owners);
+    assert.deepStrictEqual(multisigAccount.owners, owners);
     assert.ok(multisigAccount.ownerSetSeqno === 0);
 
     const pid = program.programId;
@@ -59,18 +58,17 @@ describe("multisig", () => {
       },
     ];
     const newOwners = [ownerA.publicKey, ownerB.publicKey, ownerD.publicKey];
-    const data = program.coder.instruction.encode('set_owners', {
-        owners: newOwners,
+    const data = program.coder.instruction.encode("set_owners", {
+      owners: newOwners,
     });
 
-    const transaction = new anchor.web3.Account();
+    const transaction = anchor.web3.Keypair.generate();
     const txSize = 1000; // Big enough, cuz I'm lazy.
     await program.rpc.createTransaction(pid, accounts, data, {
       accounts: {
         multisig: multisig.publicKey,
         transaction: transaction.publicKey,
         proposer: ownerA.publicKey,
-        rent: anchor.web3.SYSVAR_RENT_PUBKEY,
       },
       instructions: [
         await program.account.transaction.createInstruction(
@@ -81,13 +79,15 @@ describe("multisig", () => {
       signers: [transaction, ownerA],
     });
 
-    const txAccount = await program.account.transaction(transaction.publicKey);
+    const txAccount = await program.account.transaction.fetch(
+      transaction.publicKey
+    );
 
     assert.ok(txAccount.programId.equals(pid));
-    assert.deepEqual(txAccount.accounts, accounts);
-    assert.deepEqual(txAccount.data, data);
+    assert.deepStrictEqual(txAccount.accounts, accounts);
+    assert.deepStrictEqual(txAccount.data, data);
     assert.ok(txAccount.multisig.equals(multisig.publicKey));
-    assert.equal(txAccount.didExecute, false);
+    assert.deepStrictEqual(txAccount.didExecute, false);
     assert.ok(txAccount.ownerSetSeqno === 0);
 
     // Other owner approves transactoin.
@@ -125,11 +125,47 @@ describe("multisig", () => {
         }),
     });
 
-    multisigAccount = await program.account.multisig(multisig.publicKey);
+    multisigAccount = await program.account.multisig.fetch(multisig.publicKey);
 
-    assert.equal(multisigAccount.nonce, nonce);
+    assert.strictEqual(multisigAccount.nonce, nonce);
     assert.ok(multisigAccount.threshold.eq(new anchor.BN(2)));
-    assert.deepEqual(multisigAccount.owners, newOwners);
+    assert.deepStrictEqual(multisigAccount.owners, newOwners);
     assert.ok(multisigAccount.ownerSetSeqno === 1);
   });
+
+  it("Assert Unique Owners", async () => {
+    const multisig = anchor.web3.Keypair.generate();
+    const [_multisigSigner, nonce] =
+      await anchor.web3.PublicKey.findProgramAddress(
+        [multisig.publicKey.toBuffer()],
+        program.programId
+      );
+    const multisigSize = 200; // Big enough.
+
+    const ownerA = anchor.web3.Keypair.generate();
+    const ownerB = anchor.web3.Keypair.generate();
+    const owners = [ownerA.publicKey, ownerB.publicKey, ownerA.publicKey];
+
+    const threshold = new anchor.BN(2);
+    try {
+      await program.rpc.createMultisig(owners, threshold, nonce, {
+        accounts: {
+          multisig: multisig.publicKey,
+          rent: anchor.web3.SYSVAR_RENT_PUBKEY,
+        },
+        instructions: [
+          await program.account.multisig.createInstruction(
+            multisig,
+            multisigSize
+          ),
+        ],
+        signers: [multisig],
+      });
+      assert.fail();
+    } catch (err) {
+      const error = err.error;
+      assert.strictEqual(error.errorCode.number, 6008);
+      assert.strictEqual(error.errorMessage, "Owners must be unique");
+    }
+  });
 });