examples/multisig: Changes for upgradeable programs (#98)

examples/multisig/programs/multisig/src/lib.rs

@@ -1,4 +1,21 @@
 //! An example of a multisig to execute arbitrary Solana transactions.
+//!
+//! This program can be used to allow a multisig to govern anything a regular
+//! Pubkey can govern. One can use the multisig as a BPF program upgrade
+//! authority, a mint authority, etc.
+//!
+//! To use, one must first create a `Multisig` account, sepcifying two important
+//! parameters:
+//!
+//! 1. Owners - the set of addresses that sign transactions for the multisig.
+//! 2. Threhsold - the number of signers required to execute a transaction.
+//!
+//! Once the `Multisig` account is created, once can create a `Transaction`
+//! account, specifying the parameters for a normal solana transaction.
+//!
+//! To sign, owners shold invoke the `approve` instruction, and finally,
+//! the `execute_transaction`, once enough (i.e. `threhsold`) of the owners have
+//! signed.
 
 #![feature(proc_macro_hygiene)]
 
@@ -11,6 +28,7 @@ use std::convert::Into;
 pub mod multisig {
     use super::*;
 
+    // Initializes a new multisig account with a set of owners and a threshold.
     pub fn create_multisig(
         ctx: Context<CreateMultisig>,
         owners: Vec<Pubkey>,
@@ -24,6 +42,8 @@ pub mod multisig {
         Ok(())
     }
 
+    // Creates a new transaction account, automatically signed by the creator,
+    // which must be one of the owners of the multisig.
     pub fn create_transaction(
         ctx: Context<CreateTransaction>,
         pid: Pubkey,
@@ -53,6 +73,55 @@ pub mod multisig {
         Ok(())
     }
 
+    // Deletes a transaction if no one has signed, other than the creator.
+    // Note that the account is not actually deleted until the next epoch,
+    // when the account is garbage collected since there's no more SOL left.
+    // As a result, during the period of time when the transaction is deleted
+    // but the next epoch hasn't occured, the transaction account can still
+    // be used. To get around this, one could just add a field to mark deletion.
+    // This is not currently done.
+    pub fn delete_transaction(ctx: Context<DeleteTransaction>) -> Result<()> {
+        // Check the creator is the one and only signer.
+        let num_signers = ctx
+            .accounts
+            .transaction
+            .signers
+            .iter()
+            .fold(0, |acc, did_sign| if *did_sign { acc + 1 } else { acc });
+        if num_signers != 1 {
+            return Err(ErrorCode::TransactionAlreadySigned.into());
+        }
+        let owner_index = ctx
+            .accounts
+            .multisig
+            .owners
+            .iter()
+            .position(|a| a == ctx.accounts.owner.key)
+            .ok_or(ErrorCode::InvalidOwner)?;
+        if !ctx.accounts.transaction.signers[owner_index] {
+            return Err(ErrorCode::UnableToDelete.into());
+        }
+
+        // Mark the creator as not signed.
+        ctx.accounts.transaction.signers[owner_index] = false;
+
+        // Send the rent exemption sol back to the creator of the transaction.
+        let tx_lamports = ctx.accounts.transaction.to_account_info().lamports();
+        let to_lamports = ctx.accounts.owner.lamports();
+        **ctx.accounts.owner.lamports.borrow_mut() = to_lamports
+            .checked_add(tx_lamports)
+            .ok_or(ErrorCode::Overflow)?;
+        **ctx
+            .accounts
+            .transaction
+            .to_account_info()
+            .lamports
+            .borrow_mut() = 0;
+
+        Ok(())
+    }
+
+    // Approves a transaction on behalf of an owner of the multisig.
     pub fn approve(ctx: Context<Approve>) -> Result<()> {
         let owner_index = ctx
             .accounts
@@ -80,14 +149,26 @@ pub mod multisig {
         Ok(())
     }
 
+    // Changes the execution threshold of the multisig. The only way this can be
+    // invoked is via a recursive call from execute_transaction ->
+    // change_threshold.
     pub fn change_threshold(ctx: Context<Auth>, threshold: u64) -> Result<()> {
+        if threshold > ctx.accounts.multisig.owners.len() as u64 {
+            return Err(ErrorCode::InvalidThreshold.into());
+        }
         let multisig = &mut ctx.accounts.multisig;
         multisig.threshold = threshold;
         Ok(())
     }
 
+    // Executes the given transaction if threshold owners have signed it.
     pub fn execute_transaction(ctx: Context<ExecuteTransaction>) -> Result<()> {
-        // Check we have enough signers.
+        // Has this been executed already?
+        if ctx.accounts.transaction.did_execute {
+            return Err(ErrorCode::AlreadyExecuted.into());
+        }
+
+        // Do we have enough signers.
         let sig_count = ctx
             .accounts
             .transaction
@@ -103,8 +184,19 @@ pub mod multisig {
             return Err(ErrorCode::NotEnoughSigners.into());
         }
 
-        // Execute the multisig transaction.
-        let ix: Instruction = (&*ctx.accounts.transaction).into();
+        // Execute the transaction signed by the multisig.
+        let mut ix: Instruction = (&*ctx.accounts.transaction).into();
+        ix.accounts = ix
+            .accounts
+            .iter()
+            .map(|acc| {
+                if &acc.pubkey == ctx.accounts.multisig_signer.key {
+                    AccountMeta::new_readonly(acc.pubkey, true)
+                } else {
+                    acc.clone()
+                }
+            })
+            .collect();
         let seeds = &[
             ctx.accounts.multisig.to_account_info().key.as_ref(),
             &[ctx.accounts.multisig.nonce],
@@ -113,7 +205,7 @@ pub mod multisig {
         let accounts = ctx.remaining_accounts;
         solana_program::program::invoke_signed(&ix, &accounts, signer)?;
 
-        // Burn the account to ensure one time use.
+        // Burn the transaction to ensure one time use.
         ctx.accounts.transaction.did_execute = true;
 
         Ok(())
@@ -132,17 +224,28 @@ pub struct CreateTransaction<'info> {
     multisig: ProgramAccount<'info, Multisig>,
     #[account(init)]
     transaction: ProgramAccount<'info, Transaction>,
+    // One of the owners. Checked in the handler.
     #[account(signer)]
     proposer: AccountInfo<'info>,
     rent: Sysvar<'info, Rent>,
 }
 
+#[derive(Accounts)]
+pub struct DeleteTransaction<'info> {
+    multisig: ProgramAccount<'info, Multisig>,
+    #[account(belongs_to = multisig)]
+    transaction: ProgramAccount<'info, Transaction>,
+    // One of the multisig owners. Checked in the handler.
+    #[account(signer)]
+    owner: AccountInfo<'info>,
+}
+
 #[derive(Accounts)]
 pub struct Approve<'info> {
     multisig: ProgramAccount<'info, Multisig>,
     #[account(mut, belongs_to = multisig)]
     transaction: ProgramAccount<'info, Transaction>,
-    // One of the multisig owners.
+    // One of the multisig owners. Checked in the handler.
     #[account(signer)]
     owner: AccountInfo<'info>,
 }
@@ -179,6 +282,8 @@ pub struct Multisig {
 
 #[account]
 pub struct Transaction {
+    // The multisig account this transaction belongs to.
+    multisig: Pubkey,
     // Target program to execute against.
     program_id: Pubkey,
     // Accounts requried for the transaction.
@@ -187,8 +292,6 @@ pub struct Transaction {
     data: Vec<u8>,
     // signers[index] is true iff multisig.owners[index] signed the transaction.
     signers: Vec<bool>,
-    // The multisig account this transaction belongs to.
-    multisig: Pubkey,
     // Boolean ensuring one time execution.
     did_execute: bool,
 }
@@ -225,5 +328,14 @@ pub enum ErrorCode {
     InvalidOwner,
     #[msg("Not enough owners signed this transaction.")]
     NotEnoughSigners,
-    Unknown,
+    #[msg("Cannot delete a transaction that has been signed by an owner.")]
+    TransactionAlreadySigned,
+    #[msg("Overflow when adding.")]
+    Overflow,
+    #[msg("Cannot delete a transaction the owner did not create.")]
+    UnableToDelete,
+    #[msg("The given transaction has already been executed.")]
+    AlreadyExecuted,
+    #[msg("Threshold must be less than or equal to the number of owners.")]
+    InvalidThreshold,
 }

examples/multisig/tests/multisig.js

@@ -21,6 +21,7 @@ describe("multisig", () => {
     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 owners = [ownerA.publicKey, ownerB.publicKey, ownerC.publicKey];
 
     const threshold = new anchor.BN(2);
@@ -57,7 +58,7 @@ describe("multisig", () => {
         isSigner: true,
       },
     ];
-    const newOwners = [ownerA.publicKey, ownerB.publicKey];
+    const newOwners = [ownerA.publicKey, ownerB.publicKey, ownerD.publicKey];
     const data = program.coder.instruction.encode('set_owners', {
         owners: newOwners,
     });