Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add check on transaction creation #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 19 additions & 20 deletions programs/multisig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ pub mod coral_multisig {
accs: Vec<TransactionAccount>,
data: Vec<u8>,
) -> Result<()> {
// Refuses transaction that do not include the multisig signer account.
let multisig_signer = accs
.iter()
.find(|a| a.pubkey == *ctx.accounts.multisig_signer.key)
.ok_or(ErrorCode::TransactionMissingMultisig)?;
require!(multisig_signer.is_signer, ErrorCode::MultisigNotSigner);

let owner_index = ctx
.accounts
.multisig
Expand Down Expand Up @@ -166,19 +173,7 @@ pub mod coral_multisig {
return Err(ErrorCode::NotEnoughSigners.into());
}

// 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[..]];
Expand All @@ -203,6 +198,12 @@ pub struct CreateTransaction<'info> {
multisig: Box<Account<'info, Multisig>>,
#[account(zero, signer)]
transaction: Box<Account<'info, Transaction>>,
/// 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>,
// One of the owners. Checked in the handler.
proposer: Signer<'info>,
}
Expand Down Expand Up @@ -232,12 +233,6 @@ pub struct Auth<'info> {
pub struct ExecuteTransaction<'info> {
#[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>>,
}
Expand Down Expand Up @@ -332,6 +327,10 @@ pub enum ErrorCode {
AlreadyExecuted,
#[msg("Threshold must be less than or equal to the number of owners.")]
InvalidThreshold,
#[msg("Owners must be unique")]
#[msg("Owners must be unique.")]
UniqueOwners,
#[msg("Transaction does not include the multisig signer account.")]
TransactionMissingMultisig,
#[msg("Multisig PDA is not a signer.")]
MultisigNotSigner,
}
6 changes: 3 additions & 3 deletions tests/multisig.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ describe("multisig", () => {
multisig: multisig.publicKey,
transaction: transaction.publicKey,
proposer: ownerA.publicKey,
multisigSigner
},
instructions: [
await program.account.transaction.createInstruction(
Expand Down Expand Up @@ -104,7 +105,6 @@ describe("multisig", () => {
await program.rpc.executeTransaction({
accounts: {
multisig: multisig.publicKey,
multisigSigner,
transaction: transaction.publicKey,
},
remainingAccounts: program.instruction.setOwners
Expand Down Expand Up @@ -165,7 +165,7 @@ describe("multisig", () => {
} catch (err) {
const error = err.error;
assert.strictEqual(error.errorCode.number, 6008);
assert.strictEqual(error.errorMessage, "Owners must be unique");
assert.strictEqual(error.errorMessage, "Owners must be unique.");
}
});
});
});