diff --git a/programs/native/vote/src/lib.rs b/programs/native/vote/src/lib.rs index b981b696b9..a2f55c1f34 100644 --- a/programs/native/vote/src/lib.rs +++ b/programs/native/vote/src/lib.rs @@ -21,12 +21,6 @@ fn entrypoint( trace!("process_instruction: {:?}", data); trace!("keyed_accounts: {:?}", keyed_accounts); - // all vote instructions require that accounts_keys[0] be a signer - if keyed_accounts[0].signer_key().is_none() { - error!("account[0] is unsigned"); - Err(ProgramError::InvalidArgument)?; - } - match deserialize(data).map_err(|_| ProgramError::InvalidUserdata)? { VoteInstruction::InitializeAccount => vote_program::initialize_account(keyed_accounts), VoteInstruction::DelegateStake(delegate_id) => { diff --git a/programs/native/vote/tests/vote.rs b/programs/native/vote/tests/vote.rs new file mode 100644 index 0000000000..2bb5216f27 --- /dev/null +++ b/programs/native/vote/tests/vote.rs @@ -0,0 +1,100 @@ +//use solana_runtime::bank::BankError; +use solana_runtime::bank::{Bank, Result}; +use solana_sdk::genesis_block::GenesisBlock; +use solana_sdk::hash::hash; +//use solana_sdk::native_program::ProgramError; +use solana_sdk::pubkey::Pubkey; +use solana_sdk::signature::{Keypair, KeypairUtil}; +use solana_sdk::system_instruction::SystemInstruction; +use solana_sdk::transaction_builder::{BuilderInstruction, TransactionBuilder}; +use solana_sdk::vote_program::{self, Vote, VoteInstruction, VoteState}; +use solana_sdk::vote_transaction::VoteTransaction; + +struct VoteBank<'a> { + bank: &'a Bank, +} + +impl<'a> VoteBank<'a> { + fn new(bank: &'a Bank) -> Self { + bank.add_native_program("solana_vote_program", &vote_program::id()); + Self { bank } + } + + fn create_vote_account( + &self, + from_keypair: &Keypair, + vote_id: Pubkey, + lamports: u64, + ) -> Result<()> { + let last_id = self.bank.last_id(); + let tx = VoteTransaction::fund_staking_account(from_keypair, vote_id, last_id, lamports, 0); + self.bank.process_transaction(&tx) + } + + fn submit_vote(&self, vote_keypair: &Keypair, tick_height: u64) -> Result { + let last_id = self.bank.last_id(); + let tx = VoteTransaction::new_vote(vote_keypair, tick_height, last_id, 0); + self.bank.process_transaction(&tx)?; + self.bank.register_tick(&hash(last_id.as_ref())); + + let vote_account = self.bank.get_account(&vote_keypair.pubkey()).unwrap(); + Ok(VoteState::deserialize(&vote_account.userdata).unwrap()) + } +} + +#[test] +fn test_vote_via_bank() { + let (genesis_block, from_keypair) = GenesisBlock::new(10_000); + let bank = Bank::new(&genesis_block); + let vote_bank = VoteBank::new(&bank); + + let vote_keypair = Keypair::new(); + let vote_id = vote_keypair.pubkey(); + vote_bank + .create_vote_account(&from_keypair, vote_id, 100) + .unwrap(); + + let vote_state = vote_bank.submit_vote(&vote_keypair, 0).unwrap(); + assert_eq!(vote_state.votes.len(), 1); +} + +#[test] +fn test_vote_via_bank_with_no_signature() { + let (genesis_block, mallory_keypair) = GenesisBlock::new(10_000); + let bank = Bank::new(&genesis_block); + let vote_bank = VoteBank::new(&bank); + + let vote_keypair = Keypair::new(); + let vote_id = vote_keypair.pubkey(); + vote_bank + .create_vote_account(&mallory_keypair, vote_id, 100) + .unwrap(); + + let mallory_id = mallory_keypair.pubkey(); + let last_id = bank.last_id(); + let vote_ix = BuilderInstruction::new( + vote_program::id(), + &VoteInstruction::Vote(Vote::new(0)), + vec![(vote_id, false)], // <--- attack!! No signature. + ); + + // Sneak in an instruction so that the transaction is signed but + // the 0th account in the second instruction is not! The program + // needs to check that it's signed. + let tx = TransactionBuilder::default() + .push(SystemInstruction::new_move(mallory_id, vote_id, 1)) + .push(vote_ix) + .sign(&[&mallory_keypair], last_id); + + let _result = bank.process_transaction(&tx); + + // And ensure there's no vote. + let vote_account = bank.get_account(&vote_id).unwrap(); + let vote_state = VoteState::deserialize(&vote_account.userdata).unwrap(); + assert_eq!(vote_state.votes.len(), 0); + + //assert_eq!( + // result, + // Err(BankError::ProgramError(1, ProgramError::InvalidArgument)) + //); +} diff --git a/sdk/src/transaction_builder.rs b/sdk/src/transaction_builder.rs index ac43e8a41b..aeb6615a3d 100644 --- a/sdk/src/transaction_builder.rs +++ b/sdk/src/transaction_builder.rs @@ -53,14 +53,24 @@ impl TransactionBuilder { /// Return pubkeys referenced by all instructions, with the ones needing signatures first. /// No duplicates and order is preserved. - fn keys(&self) -> Vec { - let mut key_and_signed: Vec<_> = self + fn keys(&self) -> (Vec, Vec) { + let mut keys_and_signed: Vec<_> = self .instructions .iter() .flat_map(|ix| ix.accounts.iter()) .collect(); - key_and_signed.sort_by(|x, y| y.1.cmp(&x.1)); - key_and_signed.into_iter().map(|x| x.0).unique().collect() + keys_and_signed.sort_by(|x, y| y.1.cmp(&x.1)); + + let mut signed_keys = vec![]; + let mut unsigned_keys = vec![]; + for (key, signed) in keys_and_signed.into_iter().unique_by(|x| x.0) { + if *signed { + signed_keys.push(*key); + } else { + unsigned_keys.push(*key); + } + } + (signed_keys, unsigned_keys) } /// Return program ids referenced by all instructions. No duplicates and order is preserved. @@ -82,16 +92,18 @@ impl TransactionBuilder { /// Return a signed transaction. pub fn sign(&self, keypairs: &[&T], last_id: Hash) -> Transaction { - let keys = self.keys(); let program_ids = self.program_ids(); - let instructions = self.instructions(&keys, &program_ids); + let (mut signed_keys, unsigned_keys) = self.keys(); for (i, keypair) in keypairs.iter().enumerate() { - assert_eq!(keypair.pubkey(), keys[i], "keypair-pubkey mismatch"); + assert_eq!(keypair.pubkey(), signed_keys[i], "keypair-pubkey mismatch"); } - let unsigned_keys = &keys[keypairs.len()..]; + assert_eq!(keypairs.len(), signed_keys.len(), "not enough keypairs"); + + signed_keys.extend(&unsigned_keys); + let instructions = self.instructions(&signed_keys, &program_ids); Transaction::new_with_instructions( keypairs, - unsigned_keys, + &unsigned_keys, last_id, self.fee, program_ids, @@ -147,7 +159,7 @@ mod tests { .push(Instruction::new(program_id, &0, vec![(id0, true)])) .push(Instruction::new(program_id, &0, vec![(id0, true)])) .keys(); - assert_eq!(keys, vec![id0]); + assert_eq!(keys, (vec![id0], vec![])); } #[test] @@ -158,7 +170,7 @@ mod tests { .push(Instruction::new(program_id, &0, vec![(id0, false)])) .push(Instruction::new(program_id, &0, vec![(id0, true)])) .keys(); - assert_eq!(keys, vec![id0]); + assert_eq!(keys, (vec![id0], vec![])); } #[test] @@ -170,7 +182,7 @@ mod tests { .push(Instruction::new(program_id, &0, vec![(id0, false)])) .push(Instruction::new(program_id, &0, vec![(id1, false)])) .keys(); - assert_eq!(keys, vec![id0, id1]); + assert_eq!(keys, (vec![], vec![id0, id1])); } #[test] @@ -183,7 +195,7 @@ mod tests { .push(Instruction::new(program_id, &0, vec![(id1, false)])) .push(Instruction::new(program_id, &0, vec![(id0, true)])) .keys(); - assert_eq!(keys, vec![id0, id1]); + assert_eq!(keys, (vec![id0], vec![id1])); } #[test] @@ -195,7 +207,7 @@ mod tests { .push(Instruction::new(program_id, &0, vec![(id0, false)])) .push(Instruction::new(program_id, &0, vec![(id1, true)])) .keys(); - assert_eq!(keys, vec![id1, id0]); + assert_eq!(keys, (vec![id1], vec![id0])); } #[test] @@ -205,6 +217,17 @@ mod tests { TransactionBuilder::default().sign(&[&keypair], Hash::default()); } + #[test] + #[should_panic] + fn test_transaction_builder_missing_keypair() { + let program_id = Pubkey::default(); + let keypair0 = Keypair::new(); + let id0 = keypair0.pubkey(); + TransactionBuilder::default() + .push(Instruction::new(program_id, &0, vec![(id0, true)])) + .sign(&Vec::<&Keypair>::new(), Hash::default()); + } + #[test] #[should_panic] fn test_transaction_builder_wrong_key() { diff --git a/sdk/src/vote_program.rs b/sdk/src/vote_program.rs index 47757f14b4..f5bc8331f5 100644 --- a/sdk/src/vote_program.rs +++ b/sdk/src/vote_program.rs @@ -272,6 +272,11 @@ pub fn process_vote(keyed_accounts: &mut [KeyedAccount], vote: Vote) -> Result<( Err(ProgramError::InvalidArgument)?; } + if keyed_accounts[0].signer_key().is_none() { + error!("account[0] should sign the transaction"); + Err(ProgramError::InvalidArgument)?; + } + let mut vote_state = VoteState::deserialize(&keyed_accounts[0].account.userdata)?; vote_state.process_vote(vote); vote_state.serialize(&mut keyed_accounts[0].account.userdata)?; @@ -403,6 +408,21 @@ mod tests { assert_eq!(vote_state.credits(), 0); } + #[test] + fn test_vote_signature() { + let from_id = Keypair::new().pubkey(); + let mut from_account = Account::new(100, 0, Pubkey::default()); + let vote_id = Keypair::new().pubkey(); + let mut vote_account = create_vote_account(100); + initialize_and_deserialize(&from_id, &mut from_account, &vote_id, &mut vote_account) + .unwrap(); + + let vote = Vote::new(1); + let mut keyed_accounts = [KeyedAccount::new(&vote_id, false, &mut vote_account)]; + let res = process_vote(&mut keyed_accounts, vote); + assert_eq!(res, Err(ProgramError::InvalidArgument)); + } + #[test] fn test_vote_without_initialization() { let vote_id = Keypair::new().pubkey();