From e7de7c32dbb74e411421f1a76e48e18c2e0223bd Mon Sep 17 00:00:00 2001 From: anatoly yakovenko Date: Fri, 28 Sep 2018 16:16:35 -0700 Subject: [PATCH] Transactions with multiple programs. (#1381) Transactions contain a vector of instructions that are executed atomically. Bench shows a 2.3x speed up when using 5 instructions per tx. --- benches/banking_stage.rs | 93 ++++++++++++- src/bank.rs | 273 +++++++++++++++++++++++++++----------- src/banking_stage.rs | 2 +- src/budget_program.rs | 123 +++++++++-------- src/budget_transaction.rs | 57 +++++--- src/mint.rs | 5 +- src/rpc.rs | 2 +- src/storage_program.rs | 25 +++- src/system_program.rs | 45 ++++--- src/system_transaction.rs | 26 ++-- src/thin_client.rs | 4 +- src/tictactoe_program.rs | 15 ++- src/transaction.rs | 235 +++++++++++++++++++++++++------- tests/programs.rs | 12 +- 14 files changed, 668 insertions(+), 249 deletions(-) diff --git a/benches/banking_stage.rs b/benches/banking_stage.rs index 8053c3043c..0991a3735d 100644 --- a/benches/banking_stage.rs +++ b/benches/banking_stage.rs @@ -63,8 +63,8 @@ fn bench_banking_stage_multi_accounts(bencher: &mut Bencher) { let from: Vec = (0..64).map(|_| thread_rng().gen()).collect(); let to: Vec = (0..64).map(|_| thread_rng().gen()).collect(); let sig: Vec = (0..64).map(|_| thread_rng().gen()).collect(); - new.keys[0] = Pubkey::new(&from[0..32]); - new.keys[1] = Pubkey::new(&to[0..32]); + new.account_keys[0] = Pubkey::new(&from[0..32]); + new.account_keys[1] = Pubkey::new(&to[0..32]); new.signature = Signature::new(&sig[0..64]); new }).collect(); @@ -72,7 +72,7 @@ fn bench_banking_stage_multi_accounts(bencher: &mut Bencher) { transactions.iter().for_each(|tx| { let fund = Transaction::system_move( &mint.keypair(), - tx.keys[0], + tx.account_keys[0], mint_total / txes as i64, mint.last_id(), 0, @@ -108,3 +108,90 @@ fn bench_banking_stage_multi_accounts(bencher: &mut Bencher) { bank.register_entry_id(&mint.last_id()); }); } + +#[bench] +fn bench_banking_stage_multi_programs(bencher: &mut Bencher) { + //use solana::logger; + //logger::setup(); + let progs = 5; + let txes = 1000 * NUM_THREADS; + let mint_total = 1_000_000_000_000; + let mint = Mint::new(mint_total); + + let (verified_sender, verified_receiver) = channel(); + let bank = Arc::new(Bank::new(&mint)); + let dummy = Transaction::system_move( + &mint.keypair(), + mint.keypair().pubkey(), + 1, + mint.last_id(), + 0, + ); + let transactions: Vec<_> = (0..txes) + .into_par_iter() + .map(|_| { + let mut new = dummy.clone(); + let from: Vec = (0..32).map(|_| thread_rng().gen()).collect(); + let sig: Vec = (0..64).map(|_| thread_rng().gen()).collect(); + let to: Vec = (0..32).map(|_| thread_rng().gen()).collect(); + new.account_keys[0] = Pubkey::new(&from[0..32]); + new.account_keys[1] = Pubkey::new(&to[0..32]); + let prog = new.instructions[0].clone(); + for i in 1..progs { + //generate programs that spend to random keys + let to: Vec = (0..32).map(|_| thread_rng().gen()).collect(); + let to_key = Pubkey::new(&to[0..32]); + new.account_keys.push(to_key); + assert_eq!(new.account_keys.len(), i + 2); + new.instructions.push(prog.clone()); + assert_eq!(new.instructions.len(), i + 1); + new.instructions[i].accounts[1] = 1 + i as u8; + assert_eq!(new.key(i, 1), Some(&to_key)); + assert_eq!( + new.account_keys[new.instructions[i].accounts[1] as usize], + to_key + ); + } + assert_eq!(new.instructions.len(), progs); + new.signature = Signature::new(&sig[0..64]); + new + }).collect(); + transactions.iter().for_each(|tx| { + let fund = Transaction::system_move( + &mint.keypair(), + tx.account_keys[0], + mint_total / txes as i64, + mint.last_id(), + 0, + ); + assert!(bank.process_transaction(&fund).is_ok()); + }); + //sanity check, make sure all the transactions can execute sequentially + transactions.iter().for_each(|tx| { + let res = bank.process_transaction(&tx); + assert!(res.is_ok(), "sanity test transactions"); + }); + bank.clear_signatures(); + //sanity check, make sure all the transactions can execute in parallel + let res = bank.process_transactions(&transactions); + for r in res { + assert!(r.is_ok(), "sanity parallel execution"); + } + bank.clear_signatures(); + let verified: Vec<_> = to_packets_chunked(&transactions.clone(), 96) + .into_iter() + .map(|x| { + let len = x.read().unwrap().packets.len(); + (x, iter::repeat(1).take(len).collect()) + }).collect(); + let (_stage, signal_receiver) = BankingStage::new(&bank, verified_receiver, Default::default()); + bencher.iter(move || { + for v in verified.chunks(verified.len() / NUM_THREADS) { + verified_sender.send(v.to_vec()).unwrap(); + } + check_txs(&signal_receiver, txes); + bank.clear_signatures(); + // make sure the transactions are still valid + bank.register_entry_id(&mint.last_id()); + }); +} diff --git a/src/bank.rs b/src/bank.rs index c40b527b6a..744cc32717 100644 --- a/src/bank.rs +++ b/src/bank.rs @@ -6,7 +6,6 @@ use bincode::deserialize; use bincode::serialize; use budget_program::BudgetState; -use budget_transaction::BudgetTransaction; use counter::Counter; use dynamic_program::DynamicProgram; use entry::Entry; @@ -68,23 +67,23 @@ pub enum BankError { /// Proof of History verification failed. LedgerVerificationFailed, /// Contract's transaction token balance does not equal the balance after the transaction - UnbalancedTransaction, + UnbalancedTransaction(u8), /// Contract's transactions resulted in an account with a negative balance /// The difference from InsufficientFundsForFee is that the transaction was executed by the /// contract - ResultWithNegativeTokens, + ResultWithNegativeTokens(u8), /// Contract id is unknown - UnknownContractId, + UnknownContractId(u8), /// Contract modified an accounts contract id - ModifiedContractId, + ModifiedContractId(u8), /// Contract spent the tokens of an account that doesn't belong to it - ExternalAccountTokenSpend, + ExternalAccountTokenSpend(u8), /// The program returned an error - ProgramRuntimeError, + ProgramRuntimeError(u8), } pub type Result = result::Result; @@ -94,7 +93,6 @@ type SignatureStatusMap = HashMap>; struct ErrorCounters { account_not_found_validator: usize, account_not_found_leader: usize, - account_not_found_vote: usize, } /// The state of all accounts and contracts after processing its entries. @@ -294,24 +292,18 @@ impl Bank { error_counters: &mut ErrorCounters, ) -> Result> { // Copy all the accounts - if accounts.get(&tx.keys[0]).is_none() { + if accounts.get(&tx.account_keys[0]).is_none() { if !self.is_leader { error_counters.account_not_found_validator += 1; } else { error_counters.account_not_found_leader += 1; } - if BudgetState::check_id(&tx.program_id) { - use budget_instruction::Instruction; - if let Some(Instruction::NewVote(_vote)) = tx.instruction() { - error_counters.account_not_found_vote += 1; - } - } Err(BankError::AccountNotFound) - } else if accounts.get(&tx.keys[0]).unwrap().tokens < tx.fee { + } else if accounts.get(&tx.account_keys[0]).unwrap().tokens < tx.fee { Err(BankError::InsufficientFundsForFee) } else { let mut called_accounts: Vec = tx - .keys + .account_keys .iter() .map(|key| accounts.get(key).cloned().unwrap_or_default()) .collect(); @@ -335,7 +327,8 @@ impl Bank { } pub fn verify_transaction( - tx: &Transaction, + program_index: usize, + tx_program_id: &Pubkey, pre_program_id: &Pubkey, pre_tokens: i64, account: &Account, @@ -343,84 +336,138 @@ impl Bank { // Verify the transaction // make sure that program_id is still the same or this was just assigned by the system call contract if !((*pre_program_id == account.program_id) - || (SystemProgram::check_id(&tx.program_id) + || (SystemProgram::check_id(&tx_program_id) && SystemProgram::check_id(&pre_program_id))) { //TODO, this maybe redundant bpf should be able to guarantee this property - return Err(BankError::ModifiedContractId); + return Err(BankError::ModifiedContractId(program_index as u8)); } // For accounts unassigned to the contract, the individual balance of each accounts cannot decrease. - if tx.program_id != account.program_id && pre_tokens > account.tokens { - return Err(BankError::ExternalAccountTokenSpend); + if *tx_program_id != account.program_id && pre_tokens > account.tokens { + return Err(BankError::ExternalAccountTokenSpend(program_index as u8)); } if account.tokens < 0 { - return Err(BankError::ResultWithNegativeTokens); + return Err(BankError::ResultWithNegativeTokens(program_index as u8)); } Ok(()) } - fn loaded_contract(&self, tx: &Transaction, accounts: &mut [Account]) -> bool { + fn loaded_contract( + &self, + tx_program_id: &Pubkey, + tx: &Transaction, + program_index: usize, + accounts: &mut [&mut Account], + ) -> bool { let loaded_contracts = self.loaded_contracts.write().unwrap(); - match loaded_contracts.get(&tx.program_id) { + match loaded_contracts.get(&tx_program_id) { Some(dc) => { - let mut infos: Vec<_> = (&tx.keys) + let mut infos: Vec<_> = (&tx.account_keys) .into_iter() .zip(accounts) .map(|(key, account)| KeyedAccount { key, account }) .collect(); - dc.call(&mut infos, &tx.userdata); + dc.call(&mut infos, tx.userdata(program_index)); true } None => false, } } - /// Execute a transaction. - /// This method calls the contract's process_transaction method and verifies that the result of - /// the contract does not violate the bank's accounting rules. + /// Execute a function with a subset of accounts as writable references. + /// Since the subset can point to the same references, in any order there is no way + /// for the borrow checker to track them with regards to the original set. + fn with_subset(accounts: &mut [Account], ixes: &[u8], func: F) -> A + where + F: Fn(&mut [&mut Account]) -> A, + { + let mut subset: Vec<&mut Account> = ixes + .iter() + .map(|ix| { + let ptr = &mut accounts[*ix as usize] as *mut Account; + // lifetime of this unsafe is only within the scope of the closure + // there is no way to reorder them without breaking borrow checker rules + unsafe { &mut *ptr } + }).collect(); + func(&mut subset) + } + /// Execute an instruction + /// This method calls the instruction's program entry pont method and verifies that the result of + /// the call does not violate the bank's accounting rules. /// The accounts are committed back to the bank only if this function returns Ok(_). - fn execute_transaction(&self, tx: &Transaction, accounts: &mut [Account]) -> Result<()> { - let pre_total: i64 = accounts.iter().map(|a| a.tokens).sum(); - let pre_data: Vec<_> = accounts + fn execute_instruction( + &self, + tx: &Transaction, + program_index: usize, + program_accounts: &mut [&mut Account], + ) -> Result<()> { + let tx_program_id = tx.program_id(program_index); + // TODO: the runtime should be checking read/write access to memory + // we are trusting the hard coded contracts not to clobber or allocate + let pre_total: i64 = program_accounts.iter().map(|a| a.tokens).sum(); + let pre_data: Vec<_> = program_accounts .iter_mut() .map(|a| (a.program_id, a.tokens)) .collect(); // Call the contract method // It's up to the contract to implement its own rules on moving funds - if SystemProgram::check_id(&tx.program_id) { - SystemProgram::process_transaction(&tx, accounts, &self.loaded_contracts) - } else if BudgetState::check_id(&tx.program_id) { - // TODO: the runtime should be checking read/write access to memory - // we are trusting the hard coded contracts not to clobber or allocate - if BudgetState::process_transaction(&tx, accounts).is_err() { - return Err(BankError::ProgramRuntimeError); + if SystemProgram::check_id(&tx_program_id) { + SystemProgram::process_transaction( + &tx, + program_index, + program_accounts, + &self.loaded_contracts, + ) + } else if BudgetState::check_id(&tx_program_id) { + if BudgetState::process_transaction(&tx, program_index, program_accounts).is_err() { + return Err(BankError::ProgramRuntimeError(program_index as u8)); } - } else if StorageProgram::check_id(&tx.program_id) { - if StorageProgram::process_transaction(&tx, accounts).is_err() { - return Err(BankError::ProgramRuntimeError); + } else if StorageProgram::check_id(&tx_program_id) { + if StorageProgram::process_transaction(&tx, program_index, program_accounts).is_err() { + return Err(BankError::ProgramRuntimeError(program_index as u8)); } - } else if TicTacToeProgram::check_id(&tx.program_id) { - if TicTacToeProgram::process_transaction(&tx, accounts).is_err() { - return Err(BankError::ProgramRuntimeError); + } else if TicTacToeProgram::check_id(&tx_program_id) { + if TicTacToeProgram::process_transaction(&tx, program_index, program_accounts).is_err() + { + return Err(BankError::ProgramRuntimeError(program_index as u8)); } - } else if self.loaded_contract(&tx, accounts) { + } else if self.loaded_contract(tx_program_id, tx, program_index, program_accounts) { } else { - return Err(BankError::UnknownContractId); + return Err(BankError::UnknownContractId(program_index as u8)); } // Verify the transaction - for ((pre_program_id, pre_tokens), post_account) in pre_data.iter().zip(accounts.iter()) { - Self::verify_transaction(&tx, pre_program_id, *pre_tokens, post_account)?; + for ((pre_program_id, pre_tokens), post_account) in + pre_data.iter().zip(program_accounts.iter()) + { + Self::verify_transaction( + program_index, + &tx_program_id, + pre_program_id, + *pre_tokens, + post_account, + )?; } // The total sum of all the tokens in all the pages cannot change. - let post_total: i64 = accounts.iter().map(|a| a.tokens).sum(); + let post_total: i64 = program_accounts.iter().map(|a| a.tokens).sum(); if pre_total != post_total { - Err(BankError::UnbalancedTransaction) + Err(BankError::UnbalancedTransaction(program_index as u8)) } else { Ok(()) } } + /// Execute a transaction. + /// This method calls each instruction in the transaction over the set of loaded Accounts + /// The accounts are committed back to the bank only if every instruction succeeds + fn execute_transaction(&self, tx: &Transaction, tx_accounts: &mut [Account]) -> Result<()> { + for (program_index, prog) in tx.instructions.iter().enumerate() { + Self::with_subset(tx_accounts, &prog.accounts, |program_accounts| { + self.execute_instruction(tx, program_index, program_accounts) + })?; + } + Ok(()) + } pub fn store_accounts( txs: &[Transaction], @@ -435,7 +482,7 @@ impl Bank { let tx = &txs[i]; let acc = racc.as_ref().unwrap(); - for (key, account) in tx.keys.iter().zip(acc.iter()) { + for (key, account) in tx.account_keys.iter().zip(acc.iter()) { //purge if 0 if account.tokens == 0 { accounts.remove(&key); @@ -507,10 +554,6 @@ impl Bank { "bank-appy_debits-account_not_found-leader", error_counters.account_not_found_leader ); - inc_new_counter_info!( - "bank-appy_debits-vote_account_not_found", - error_counters.account_not_found_vote - ); } } let cur_tx_count = self.transaction_count.load(Ordering::Relaxed); @@ -611,8 +654,8 @@ impl Bank { .expect("invalid ledger: need at least 2 entries"); { let tx = &entry1.transactions[0]; - assert!(SystemProgram::check_id(&tx.program_id), "Invalid ledger"); - let instruction: SystemProgram = deserialize(&tx.userdata).unwrap(); + assert!(SystemProgram::check_id(tx.program_id(0)), "Invalid ledger"); + let instruction: SystemProgram = deserialize(tx.userdata(0)).unwrap(); let deposit = if let SystemProgram::Move { tokens } = instruction { Some(tokens) } else { @@ -620,7 +663,9 @@ impl Bank { }.expect("invalid ledger, needs to start with a contract"); { let mut accounts = self.accounts.write().unwrap(); - let account = accounts.entry(tx.keys[0]).or_insert_with(Account::default); + let account = accounts + .entry(tx.account_keys[0]) + .or_insert_with(Account::default); account.tokens += deposit; trace!("applied genesis payment {:?} => {:?}", deposit, account); } @@ -700,6 +745,14 @@ impl Bank { self.get_signature_status(signature) != Err(BankError::SignatureNotFound) } + pub fn get_signature(&self, last_id: &Hash, signature: &Signature) -> Option> { + self.last_ids_sigs + .read() + .unwrap() + .get(last_id) + .and_then(|sigs| sigs.0.get(signature).cloned()) + } + /// Hash the `accounts` HashMap. This represents a validator's interpretation /// of the ledger up to the `last_id`, to be sent back to the leader when voting. pub fn hash_internal_state(&self) -> Hash { @@ -732,6 +785,7 @@ mod tests { use signature::{GenKeys, KeypairUtil}; use std; use std::io::{BufReader, Cursor, Seek, SeekFrom}; + use transaction::Instruction; #[test] fn test_bank_new() { @@ -757,6 +811,84 @@ mod tests { assert_eq!(bank.transaction_count(), 2); } + #[test] + fn test_one_tx_two_out_atomic_fail() { + let mint = Mint::new(1); + let key1 = Keypair::new().pubkey(); + let key2 = Keypair::new().pubkey(); + let bank = Bank::new(&mint); + + let spend = SystemProgram::Move { tokens: 1 }; + let instructions = vec![ + Instruction { + program_id: 0, + userdata: serialize(&spend).unwrap(), + accounts: vec![0, 1], + }, + Instruction { + program_id: 0, + userdata: serialize(&spend).unwrap(), + accounts: vec![0, 2], + }, + ]; + + let t1 = Transaction::new_with_instructions( + &mint.keypair(), + &[key1, key2], + mint.last_id(), + 0, + vec![SystemProgram::id()], + instructions, + ); + let res = bank.process_transactions(&vec![t1.clone()]); + assert_eq!(res.len(), 1); + assert_eq!(res[0], Err(BankError::ResultWithNegativeTokens(1))); + assert_eq!(bank.get_balance(&mint.pubkey()), 1); + assert_eq!(bank.get_balance(&key1), 0); + assert_eq!(bank.get_balance(&key2), 0); + assert_eq!( + bank.get_signature(&t1.last_id, &t1.signature), + Some(Err(BankError::ResultWithNegativeTokens(1))) + ); + } + #[test] + fn test_one_tx_two_out_atomic_pass() { + let mint = Mint::new(2); + let key1 = Keypair::new().pubkey(); + let key2 = Keypair::new().pubkey(); + let bank = Bank::new(&mint); + + let spend = SystemProgram::Move { tokens: 1 }; + let instructions = vec![ + Instruction { + program_id: 0, + userdata: serialize(&spend).unwrap(), + accounts: vec![0, 1], + }, + Instruction { + program_id: 0, + userdata: serialize(&spend).unwrap(), + accounts: vec![0, 2], + }, + ]; + + let t1 = Transaction::new_with_instructions( + &mint.keypair(), + &[key1, key2], + mint.last_id(), + 0, + vec![SystemProgram::id()], + instructions, + ); + let res = bank.process_transactions(&vec![t1.clone()]); + assert_eq!(res.len(), 1); + assert!(res[0].is_ok()); + assert_eq!(bank.get_balance(&mint.pubkey()), 0); + assert_eq!(bank.get_balance(&key1), 1); + assert_eq!(bank.get_balance(&key2), 1); + assert_eq!(bank.get_signature(&t1.last_id, &t1.signature), Some(Ok(()))); + } + #[test] fn test_negative_tokens() { logger::setup(); @@ -765,7 +897,7 @@ mod tests { let bank = Bank::new(&mint); let res = bank.transfer(-1, &mint.keypair(), pubkey, mint.last_id()); println!("{:?}", bank.get_account(&pubkey)); - assert_matches!(res, Err(BankError::ResultWithNegativeTokens)); + assert_matches!(res, Err(BankError::ResultWithNegativeTokens(0))); assert_eq!(bank.transaction_count(), 0); } @@ -796,7 +928,7 @@ mod tests { assert!(bank.has_signature(&signature)); assert_matches!( bank.get_signature_status(&signature), - Err(BankError::ResultWithNegativeTokens) + Err(BankError::ResultWithNegativeTokens(0)) ); // The tokens didn't move, but the from address paid the transaction fee. @@ -829,7 +961,7 @@ mod tests { assert_eq!(bank.get_balance(&pubkey), 1_000); assert_matches!( bank.transfer(10_001, &mint.keypair(), pubkey, mint.last_id()), - Err(BankError::ResultWithNegativeTokens) + Err(BankError::ResultWithNegativeTokens(0)) ); assert_eq!(bank.transaction_count(), 1); @@ -1126,19 +1258,4 @@ mod tests { def_bank.set_finality(90); assert_eq!(def_bank.finality(), 90); } - - #[test] - fn test_storage_tx() { - let mint = Mint::new(1); - let bank = Bank::new(&mint); - let tx = Transaction::new( - &mint.keypair(), - &[], - StorageProgram::id(), - vec![], // <--- attack! Panic on bad userdata? - mint.last_id(), - 0, - ); - assert!(bank.process_transaction(&tx).is_err()); - } } diff --git a/src/banking_stage.rs b/src/banking_stage.rs index e0ba5c5ce5..59403fa8ae 100644 --- a/src/banking_stage.rs +++ b/src/banking_stage.rs @@ -224,7 +224,7 @@ impl BankingStage { .zip(vers) .filter_map(|(tx, ver)| match tx { None => None, - Some((tx, _addr)) => if tx.verify_plan() && ver != 0 { + Some((tx, _addr)) => if tx.verify_refs() && tx.verify_plan() && ver != 0 { Some(tx) } else { None diff --git a/src/budget_program.rs b/src/budget_program.rs index 19718ec476..bbb92f7abf 100644 --- a/src/budget_program.rs +++ b/src/budget_program.rs @@ -11,13 +11,13 @@ use transaction::Transaction; #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] pub enum BudgetError { - InsufficientFunds(Pubkey), - ContractAlreadyExists(Pubkey), - ContractNotPending(Pubkey), - SourceIsPendingContract(Pubkey), - UninitializedContract(Pubkey), + InsufficientFunds, + ContractAlreadyExists, + ContractNotPending, + SourceIsPendingContract, + UninitializedContract, NegativeTokens, - DestinationMissing(Pubkey), + DestinationMissing, FailedWitness, UserdataTooSmall, UserdataDeserializeFailure, @@ -47,19 +47,21 @@ impl BudgetState { /// will progress one step. fn apply_signature( &mut self, - keys: &[Pubkey], - account: &mut [Account], + tx: &Transaction, + program_index: usize, + account: &mut [&mut Account], ) -> Result<(), BudgetError> { let mut final_payment = None; if let Some(ref mut budget) = self.pending_budget { - budget.apply_witness(&Witness::Signature, &keys[0]); + let key = tx.key(program_index, 0).unwrap(); + budget.apply_witness(&Witness::Signature, key); final_payment = budget.final_payment(); } if let Some(payment) = final_payment { - if keys.len() < 2 || payment.to != keys[2] { + if Some(&payment.to) != tx.key(program_index, 2) { trace!("destination missing"); - return Err(BudgetError::DestinationMissing(payment.to)); + return Err(BudgetError::DestinationMissing); } self.pending_budget = None; account[1].tokens -= payment.tokens; @@ -72,22 +74,24 @@ impl BudgetState { /// will progress one step. fn apply_timestamp( &mut self, - keys: &[Pubkey], - accounts: &mut [Account], + tx: &Transaction, + program_index: usize, + accounts: &mut [&mut Account], dt: DateTime, ) -> Result<(), BudgetError> { // Check to see if any timelocked transactions can be completed. let mut final_payment = None; if let Some(ref mut budget) = self.pending_budget { - budget.apply_witness(&Witness::Timestamp(dt), &keys[0]); + let key = tx.key(program_index, 0).unwrap(); + budget.apply_witness(&Witness::Timestamp(dt), key); final_payment = budget.final_payment(); } if let Some(payment) = final_payment { - if keys.len() < 2 || payment.to != keys[2] { + if Some(&payment.to) != tx.key(program_index, 2) { trace!("destination missing"); - return Err(BudgetError::DestinationMissing(payment.to)); + return Err(BudgetError::DestinationMissing); } self.pending_budget = None; accounts[1].tokens -= payment.tokens; @@ -99,15 +103,14 @@ impl BudgetState { /// Deduct tokens from the source account if it has sufficient funds and the contract isn't /// pending fn apply_debits_to_budget_state( - tx: &Transaction, - accounts: &mut [Account], + accounts: &mut [&mut Account], instruction: &Instruction, ) -> Result<(), BudgetError> { { // if the source account userdata is not empty, this is a pending contract if !accounts[0].userdata.is_empty() { trace!("source is pending"); - return Err(BudgetError::SourceIsPendingContract(tx.keys[0])); + return Err(BudgetError::SourceIsPendingContract); } if let Instruction::NewContract(contract) = &instruction { if contract.tokens < 0 { @@ -117,7 +120,7 @@ impl BudgetState { if accounts[0].tokens < contract.tokens { trace!("insufficient funds"); - return Err(BudgetError::InsufficientFunds(tx.keys[0])); + return Err(BudgetError::InsufficientFunds); } else { accounts[0].tokens -= contract.tokens; } @@ -130,7 +133,8 @@ impl BudgetState { /// Note: It is safe to apply credits from multiple transactions in parallel. fn apply_credits_to_budget_state( tx: &Transaction, - accounts: &mut [Account], + program_index: usize, + accounts: &mut [&mut Account], instruction: &Instruction, ) -> Result<(), BudgetError> { match instruction { @@ -143,7 +147,7 @@ impl BudgetState { let existing = Self::deserialize(&accounts[1].userdata).ok(); if Some(true) == existing.map(|x| x.initialized) { trace!("contract already exists"); - Err(BudgetError::ContractAlreadyExists(tx.keys[1])) + Err(BudgetError::ContractAlreadyExists) } else { let mut state = BudgetState::default(); state.pending_budget = Some(budget); @@ -156,35 +160,35 @@ impl BudgetState { Instruction::ApplyTimestamp(dt) => { if let Ok(mut state) = Self::deserialize(&accounts[1].userdata) { if !state.is_pending() { - Err(BudgetError::ContractNotPending(tx.keys[1])) + Err(BudgetError::ContractNotPending) } else if !state.initialized { trace!("contract is uninitialized"); - Err(BudgetError::UninitializedContract(tx.keys[1])) + Err(BudgetError::UninitializedContract) } else { trace!("apply timestamp"); - state.apply_timestamp(&tx.keys, accounts, *dt)?; + state.apply_timestamp(tx, program_index, accounts, *dt)?; trace!("apply timestamp committed"); state.serialize(&mut accounts[1].userdata) } } else { - Err(BudgetError::UninitializedContract(tx.keys[1])) + Err(BudgetError::UninitializedContract) } } Instruction::ApplySignature => { if let Ok(mut state) = Self::deserialize(&accounts[1].userdata) { if !state.is_pending() { - Err(BudgetError::ContractNotPending(tx.keys[1])) + Err(BudgetError::ContractNotPending) } else if !state.initialized { trace!("contract is uninitialized"); - Err(BudgetError::UninitializedContract(tx.keys[1])) + Err(BudgetError::UninitializedContract) } else { trace!("apply signature"); - state.apply_signature(&tx.keys, accounts)?; + state.apply_signature(tx, program_index, accounts)?; trace!("apply signature committed"); state.serialize(&mut accounts[1].userdata) } } else { - Err(BudgetError::UninitializedContract(tx.keys[1])) + Err(BudgetError::UninitializedContract) } } Instruction::NewVote(_vote) => { @@ -237,14 +241,19 @@ impl BudgetState { /// be spent from this account . pub fn process_transaction( tx: &Transaction, - accounts: &mut [Account], + program_index: usize, + accounts: &mut [&mut Account], ) -> Result<(), BudgetError> { - if let Ok(instruction) = deserialize(&tx.userdata) { + if let Ok(instruction) = deserialize(tx.userdata(program_index)) { trace!("process_transaction: {:?}", instruction); - Self::apply_debits_to_budget_state(tx, accounts, &instruction) - .and_then(|_| Self::apply_credits_to_budget_state(tx, accounts, &instruction)) + Self::apply_debits_to_budget_state(accounts, &instruction).and_then(|_| { + Self::apply_credits_to_budget_state(tx, program_index, accounts, &instruction) + }) } else { - info!("Invalid transaction userdata: {:?}", tx.userdata); + info!( + "Invalid transaction userdata: {:?}", + tx.userdata(program_index) + ); Err(BudgetError::UserdataDeserializeFailure) } } @@ -275,6 +284,10 @@ mod test { use solana_program_interface::pubkey::Pubkey; use transaction::Transaction; + fn process_transaction(tx: &Transaction, accounts: &mut [Account]) -> Result<(), BudgetError> { + let mut refs: Vec<&mut Account> = accounts.iter_mut().collect(); + BudgetState::process_transaction(&tx, 0, &mut refs[..]) + } #[test] fn test_serializer() { let mut a = Account::new(0, 512, BudgetState::id()); @@ -303,16 +316,16 @@ mod test { ]; let from = Keypair::new(); let contract = Keypair::new(); - + let userdata = vec![1, 2, 3]; let tx = Transaction::new( &from, &[contract.pubkey()], BudgetState::id(), - vec![1, 2, 3], // <== garbage instruction + userdata, Hash::default(), 0, ); - assert!(BudgetState::process_transaction(&tx, &mut accounts).is_err()); + assert!(process_transaction(&tx, &mut accounts).is_err()); } #[test] @@ -340,7 +353,7 @@ mod test { 1, Hash::default(), ); - BudgetState::process_transaction(&tx, &mut accounts).unwrap(); + process_transaction(&tx, &mut accounts).unwrap(); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 1); let state = BudgetState::deserialize(&accounts[contract_account].userdata).unwrap(); @@ -355,8 +368,8 @@ mod test { Hash::default(), ); assert_eq!( - BudgetState::process_transaction(&tx, &mut accounts), - Err(BudgetError::DestinationMissing(to.pubkey())) + process_transaction(&tx, &mut accounts), + Err(BudgetError::DestinationMissing) ); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 1); @@ -374,7 +387,7 @@ mod test { dt, Hash::default(), ); - BudgetState::process_transaction(&tx, &mut accounts).unwrap(); + process_transaction(&tx, &mut accounts).unwrap(); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 0); assert_eq!(accounts[to_account].tokens, 1); @@ -384,8 +397,8 @@ mod test { // try to replay the timestamp contract assert_eq!( - BudgetState::process_transaction(&tx, &mut accounts), - Err(BudgetError::ContractNotPending(contract.pubkey())) + process_transaction(&tx, &mut accounts), + Err(BudgetError::ContractNotPending) ); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 0); @@ -415,7 +428,7 @@ mod test { 1, Hash::default(), ); - BudgetState::process_transaction(&tx, &mut accounts).unwrap(); + process_transaction(&tx, &mut accounts).unwrap(); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 1); let state = BudgetState::deserialize(&accounts[contract_account].userdata).unwrap(); @@ -426,7 +439,7 @@ mod test { Transaction::budget_new_signature(&to, contract.pubkey(), to.pubkey(), Hash::default()); // unit test hack, the `from account` is passed instead of the `to` account to avoid // creating more account vectors - BudgetState::process_transaction(&tx, &mut accounts).unwrap(); + process_transaction(&tx, &mut accounts).unwrap(); // nothing should be changed because apply witness didn't finalize a payment assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 1); @@ -440,7 +453,7 @@ mod test { from.pubkey(), Hash::default(), ); - BudgetState::process_transaction(&tx, &mut accounts).unwrap(); + process_transaction(&tx, &mut accounts).unwrap(); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 0); assert_eq!(accounts[pay_account].tokens, 1); @@ -453,8 +466,8 @@ mod test { Hash::default(), ); assert_eq!( - BudgetState::process_transaction(&tx, &mut accounts), - Err(BudgetError::ContractNotPending(contract.pubkey())) + process_transaction(&tx, &mut accounts), + Err(BudgetError::ContractNotPending) ); assert_eq!(accounts[from_account].tokens, 0); assert_eq!(accounts[contract_account].tokens, 0); @@ -482,7 +495,7 @@ mod test { Hash::default(), ); - assert!(BudgetState::process_transaction(&tx, &mut accounts).is_err()); + assert!(process_transaction(&tx, &mut accounts).is_err()); assert!(BudgetState::deserialize(&accounts[1].userdata).is_err()); let tx = Transaction::budget_new_timestamp( @@ -492,7 +505,7 @@ mod test { Utc::now(), Hash::default(), ); - assert!(BudgetState::process_transaction(&tx, &mut accounts).is_err()); + assert!(process_transaction(&tx, &mut accounts).is_err()); assert!(BudgetState::deserialize(&accounts[1].userdata).is_err()); // Success if there was no panic... @@ -517,7 +530,7 @@ mod test { let tx = Transaction::budget_new(&keypair, to, 192, Hash::default()); assert_eq!( - tx.userdata, + tx.userdata(0).to_vec(), vec![ 0, 0, 0, 0, 192, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 192, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 4, 5, 6, 7, 8, 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 8, 7, 6, 5, 4, 1, @@ -536,7 +549,7 @@ mod test { Hash::default(), ); assert_eq!( - tx.userdata, + tx.userdata(0).to_vec(), vec![ 0, 0, 0, 0, 192, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 20, 0, 0, 0, 0, 0, 0, 0, 50, 48, 49, 54, 45, 48, 55, 45, 48, 56, 84, 48, 57, 58, 49, 48, 58, 49, 49, 90, @@ -561,10 +574,10 @@ mod test { ); let mut expected_userdata = vec![1, 0, 0, 0, 20, 0, 0, 0, 0, 0, 0, 0]; expected_userdata.extend(date_iso8601.as_bytes()); - assert_eq!(tx.userdata, expected_userdata); + assert_eq!(tx.userdata(0).to_vec(), expected_userdata); // ApplySignature let tx = Transaction::budget_new_signature(&keypair, keypair.pubkey(), to, Hash::default()); - assert_eq!(tx.userdata, vec![2, 0, 0, 0]); + assert_eq!(tx.userdata(0).to_vec(), vec![2, 0, 0, 0]); } } diff --git a/src/budget_transaction.rs b/src/budget_transaction.rs index b097a3761e..e3c9df7dd5 100644 --- a/src/budget_transaction.rs +++ b/src/budget_transaction.rs @@ -62,7 +62,7 @@ pub trait BudgetTransaction { fn vote(&self) -> Option<(Pubkey, Vote, Hash)>; - fn instruction(&self) -> Option; + fn instruction(&self, program_index: usize) -> Option; fn verify_plan(&self) -> bool; } @@ -204,26 +204,33 @@ impl BudgetTransaction for Transaction { } fn vote(&self) -> Option<(Pubkey, Vote, Hash)> { - if let Some(Instruction::NewVote(vote)) = self.instruction() { - Some((*self.from(), vote, self.last_id)) + if self.instructions.len() > 1 { + error!("expecting only 1 Instruction per vote"); + None + } else if let Some(Instruction::NewVote(vote)) = self.instruction(0) { + Some((self.account_keys[0], vote, self.last_id)) } else { None } } - fn instruction(&self) -> Option { - deserialize(&self.userdata).ok() + fn instruction(&self, program_index: usize) -> Option { + deserialize(&self.userdata(program_index)).ok() } /// Verify only the payment plan. fn verify_plan(&self) -> bool { - if let Some(Instruction::NewContract(contract)) = self.instruction() { - self.fee >= 0 - && self.fee <= contract.tokens - && contract.budget.verify(contract.tokens - self.fee) - } else { - true + for pix in 0..self.instructions.len() { + if let Some(Instruction::NewContract(contract)) = self.instruction(pix) { + if !(self.fee >= 0 + && self.fee <= contract.tokens + && contract.budget.verify(contract.tokens - self.fee)) + { + return false; + } + } } + true } } @@ -232,6 +239,7 @@ mod tests { use super::*; use bincode::{deserialize, serialize}; use signature::KeypairUtil; + use transaction; #[test] fn test_claim() { @@ -269,13 +277,18 @@ mod tests { }); let instruction = Instruction::NewContract(Contract { budget, tokens: 0 }); let userdata = serialize(&instruction).unwrap(); + let instructions = vec![transaction::Instruction { + program_id: 0, + userdata, + accounts: vec![], + }]; let claim0 = Transaction { - keys: vec![], + account_keys: vec![], last_id: Default::default(), signature: Default::default(), - program_id: Default::default(), + program_keys: vec![], + instructions, fee: 0, - userdata, }; let buf = serialize(&claim0).unwrap(); let claim1: Transaction = deserialize(&buf).unwrap(); @@ -288,14 +301,14 @@ mod tests { let keypair = Keypair::new(); let pubkey = keypair.pubkey(); let mut tx = Transaction::budget_new(&keypair, pubkey, 42, zero); - let mut instruction = tx.instruction().unwrap(); + let mut instruction = tx.instruction(0).unwrap(); if let Instruction::NewContract(ref mut contract) = instruction { contract.tokens = 1_000_000; // <-- attack, part 1! if let Budget::Pay(ref mut payment) = contract.budget { payment.tokens = contract.tokens; // <-- attack, part 2! } } - tx.userdata = serialize(&instruction).unwrap(); + tx.instructions[0].userdata = serialize(&instruction).unwrap(); assert!(tx.verify_plan()); assert!(!tx.verify_signature()); } @@ -308,13 +321,13 @@ mod tests { let pubkey1 = keypair1.pubkey(); let zero = Hash::default(); let mut tx = Transaction::budget_new(&keypair0, pubkey1, 42, zero); - let mut instruction = tx.instruction(); + let mut instruction = tx.instruction(0); if let Some(Instruction::NewContract(ref mut contract)) = instruction { if let Budget::Pay(ref mut payment) = contract.budget { payment.to = thief_keypair.pubkey(); // <-- attack! } } - tx.userdata = serialize(&instruction).unwrap(); + tx.instructions[0].userdata = serialize(&instruction).unwrap(); assert!(tx.verify_plan()); assert!(!tx.verify_signature()); } @@ -325,23 +338,23 @@ mod tests { let keypair1 = Keypair::new(); let zero = Hash::default(); let mut tx = Transaction::budget_new(&keypair0, keypair1.pubkey(), 1, zero); - let mut instruction = tx.instruction().unwrap(); + let mut instruction = tx.instruction(0).unwrap(); if let Instruction::NewContract(ref mut contract) = instruction { if let Budget::Pay(ref mut payment) = contract.budget { payment.tokens = 2; // <-- attack! } } - tx.userdata = serialize(&instruction).unwrap(); + tx.instructions[0].userdata = serialize(&instruction).unwrap(); assert!(!tx.verify_plan()); // Also, ensure all branchs of the plan spend all tokens - let mut instruction = tx.instruction().unwrap(); + let mut instruction = tx.instruction(0).unwrap(); if let Instruction::NewContract(ref mut contract) = instruction { if let Budget::Pay(ref mut payment) = contract.budget { payment.tokens = 0; // <-- whoops! } } - tx.userdata = serialize(&instruction).unwrap(); + tx.instructions[0].userdata = serialize(&instruction).unwrap(); assert!(!tx.verify_plan()); } } diff --git a/src/mint.rs b/src/mint.rs index 9eb198222a..d912d57b59 100644 --- a/src/mint.rs +++ b/src/mint.rs @@ -76,8 +76,9 @@ mod tests { fn test_create_transactions() { let mut transactions = Mint::new(100).create_transactions().into_iter(); let tx = transactions.next().unwrap(); - assert!(SystemProgram::check_id(&tx.program_id)); - let instruction: SystemProgram = deserialize(&tx.userdata).unwrap(); + assert_eq!(tx.instructions.len(), 1); + assert!(SystemProgram::check_id(tx.program_id(0))); + let instruction: SystemProgram = deserialize(tx.userdata(0)).unwrap(); if let SystemProgram::Move { tokens } = instruction { assert_eq!(tokens, 100); } diff --git a/src/rpc.rs b/src/rpc.rs index 271ebda911..1a4925a4ae 100644 --- a/src/rpc.rs +++ b/src/rpc.rs @@ -173,7 +173,7 @@ impl RpcSol for RpcSolImpl { Ok( match meta.request_processor.get_signature_status(signature) { Ok(_) => RpcSignatureStatus::Confirmed, - Err(BankError::ProgramRuntimeError) => RpcSignatureStatus::ProgramRuntimeError, + Err(BankError::ProgramRuntimeError(_)) => RpcSignatureStatus::ProgramRuntimeError, Err(BankError::SignatureNotFound) => RpcSignatureStatus::SignatureNotFound, Err(err) => { trace!("mapping {:?} to GenericFailure", err); diff --git a/src/storage_program.rs b/src/storage_program.rs index 09c0aa43a0..bd175c8cc9 100644 --- a/src/storage_program.rs +++ b/src/storage_program.rs @@ -33,9 +33,10 @@ impl StorageProgram { pub fn process_transaction( tx: &Transaction, - _accounts: &mut [Account], + pix: usize, + _accounts: &mut [&mut Account], ) -> Result<(), StorageError> { - if let Ok(syscall) = deserialize(&tx.userdata) { + if let Ok(syscall) = deserialize(tx.userdata(pix)) { match syscall { StorageProgram::SubmitMiningProof { sha_state } => { info!("Mining proof submitted with state {}", sha_state[0]); @@ -49,4 +50,22 @@ impl StorageProgram { } #[cfg(test)] -mod test {} +mod test { + use super::*; + use signature::Keypair; + use signature::KeypairUtil; + + #[test] + fn test_storage_tx() { + let keypair = Keypair::new(); + let tx = Transaction::new( + &keypair, + &[], + StorageProgram::id(), + vec![], + Default::default(), + 0, + ); + assert!(StorageProgram::process_transaction(&tx, 0, &mut []).is_err()); + } +} diff --git a/src/system_program.rs b/src/system_program.rs index 429cdb95a7..46b22b7945 100644 --- a/src/system_program.rs +++ b/src/system_program.rs @@ -49,10 +49,11 @@ impl SystemProgram { } pub fn process_transaction( tx: &Transaction, - accounts: &mut [Account], + pix: usize, + accounts: &mut [&mut Account], loaded_programs: &RwLock>, ) { - if let Ok(syscall) = deserialize(&tx.userdata) { + if let Ok(syscall) = deserialize(tx.userdata(pix)) { trace!("process_transaction: {:?}", syscall); match syscall { SystemProgram::CreateAccount { @@ -91,12 +92,13 @@ impl SystemProgram { } } } else { - info!("Invalid transaction userdata: {:?}", tx.userdata); + info!("Invalid transaction userdata: {:?}", tx.userdata(pix)); } } } #[cfg(test)] mod test { + use super::*; use hash::Hash; use signature::{Keypair, KeypairUtil}; use solana_program_interface::account::Account; @@ -107,6 +109,14 @@ mod test { use system_transaction::SystemTransaction; use transaction::Transaction; + fn process_transaction( + tx: &Transaction, + accounts: &mut [Account], + loaded_programs: &RwLock>, + ) { + let mut refs: Vec<&mut Account> = accounts.iter_mut().collect(); + SystemProgram::process_transaction(&tx, 0, &mut refs[..], loaded_programs) + } #[test] fn test_create_noop() { let from = Keypair::new(); @@ -114,7 +124,7 @@ mod test { let mut accounts = vec![Account::default(), Account::default()]; let tx = Transaction::system_new(&from, to.pubkey(), 0, Hash::default()); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert_eq!(accounts[0].tokens, 0); assert_eq!(accounts[1].tokens, 0); } @@ -126,7 +136,7 @@ mod test { accounts[0].tokens = 1; let tx = Transaction::system_new(&from, to.pubkey(), 1, Hash::default()); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert_eq!(accounts[0].tokens, 0); assert_eq!(accounts[1].tokens, 1); } @@ -139,7 +149,7 @@ mod test { accounts[0].program_id = from.pubkey(); let tx = Transaction::system_new(&from, to.pubkey(), 1, Hash::default()); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert_eq!(accounts[0].tokens, 1); assert_eq!(accounts[1].tokens, 0); } @@ -151,7 +161,7 @@ mod test { let tx = Transaction::system_create(&from, to.pubkey(), Hash::default(), 0, 1, to.pubkey(), 0); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert!(accounts[0].userdata.is_empty()); assert_eq!(accounts[1].userdata.len(), 1); assert_eq!(accounts[1].program_id, to.pubkey()); @@ -172,7 +182,7 @@ mod test { 0, ); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert!(accounts[1].userdata.is_empty()); } #[test] @@ -191,7 +201,7 @@ mod test { 0, ); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert!(accounts[1].userdata.is_empty()); } #[test] @@ -210,7 +220,7 @@ mod test { 0, ); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert_eq!(accounts[1].userdata.len(), 3); } #[test] @@ -220,7 +230,7 @@ mod test { let mut accounts = vec![Account::default()]; let tx = Transaction::system_assign(&from, Hash::default(), program.pubkey(), 0); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert_eq!(accounts[0].program_id, program.pubkey()); } #[test] @@ -231,7 +241,7 @@ mod test { accounts[0].tokens = 1; let tx = Transaction::system_new(&from, to.pubkey(), 1, Hash::default()); let hash = RwLock::new(HashMap::new()); - SystemProgram::process_transaction(&tx, &mut accounts, &hash); + process_transaction(&tx, &mut accounts, &hash); assert_eq!(accounts[0].tokens, 0); assert_eq!(accounts[1].tokens, 1); } @@ -254,7 +264,7 @@ mod test { ); assert_eq!( - tx.userdata, + tx.userdata(0).to_vec(), vec![ 0, 0, 0, 0, 111, 0, 0, 0, 0, 0, 0, 0, 222, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 @@ -273,7 +283,7 @@ mod test { ); assert_eq!( - tx.userdata, + tx.userdata(0).to_vec(), vec![ 0, 0, 0, 0, 111, 0, 0, 0, 0, 0, 0, 0, 222, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 @@ -288,7 +298,7 @@ mod test { 0, ); assert_eq!( - tx.userdata, + tx.userdata(0).to_vec(), vec![ 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 @@ -297,6 +307,9 @@ mod test { // Move let tx = Transaction::system_move(&keypair, keypair.pubkey(), 123, Hash::default(), 0); - assert_eq!(tx.userdata, vec![2, 0, 0, 0, 123, 0, 0, 0, 0, 0, 0, 0]); + assert_eq!( + tx.userdata(0).to_vec(), + vec![2, 0, 0, 0, 123, 0, 0, 0, 0, 0, 0, 0] + ); } } diff --git a/src/system_transaction.rs b/src/system_transaction.rs index 54dd978544..b3e887592d 100644 --- a/src/system_transaction.rs +++ b/src/system_transaction.rs @@ -55,23 +55,25 @@ impl SystemTransaction for Transaction { space, program_id, }; + let userdata = serialize(&create).unwrap(); Transaction::new( from_keypair, &[to], SystemProgram::id(), - serialize(&create).unwrap(), + userdata, last_id, fee, ) } /// Create and sign new SystemProgram::CreateAccount transaction fn system_assign(from_keypair: &Keypair, last_id: Hash, program_id: Pubkey, fee: i64) -> Self { - let create = SystemProgram::Assign { program_id }; + let assign = SystemProgram::Assign { program_id }; + let userdata = serialize(&assign).unwrap(); Transaction::new( from_keypair, &[], SystemProgram::id(), - serialize(&create).unwrap(), + userdata, last_id, fee, ) @@ -88,12 +90,13 @@ impl SystemTransaction for Transaction { last_id: Hash, fee: i64, ) -> Self { - let create = SystemProgram::Move { tokens }; + let move_tokens = SystemProgram::Move { tokens }; + let userdata = serialize(&move_tokens).unwrap(); Transaction::new( from_keypair, &[to], SystemProgram::id(), - serialize(&create).unwrap(), + userdata, last_id, fee, ) @@ -107,11 +110,12 @@ impl SystemTransaction for Transaction { name: String, ) -> Self { let load = SystemProgram::Load { program_id, name }; + let userdata = serialize(&load).unwrap(); Transaction::new( from_keypair, &[], SystemProgram::id(), - serialize(&load).unwrap(), + userdata, last_id, fee, ) @@ -152,7 +156,7 @@ mod tests { assert_eq!(memfind(&tx_bytes, &sign_data), Some(SIGNED_DATA_OFFSET)); assert_eq!(memfind(&tx_bytes, &tx.signature.as_ref()), Some(SIG_OFFSET)); assert_eq!( - memfind(&tx_bytes, &tx.from().as_ref()), + memfind(&tx_bytes, &tx.account_keys[0].as_ref()), Some(PUB_KEY_OFFSET) ); assert!(tx.verify_signature()); @@ -161,7 +165,7 @@ mod tests { #[test] fn test_userdata_layout() { let mut tx0 = test_tx(); - tx0.userdata = vec![1, 2, 3]; + tx0.instructions[0].userdata = vec![1, 2, 3]; let sign_data0a = tx0.get_sign_data(); let tx_bytes = serialize(&tx0).unwrap(); assert!(tx_bytes.len() < PACKET_DATA_SIZE); @@ -171,14 +175,14 @@ mod tests { Some(SIG_OFFSET) ); assert_eq!( - memfind(&tx_bytes, &tx0.from().as_ref()), + memfind(&tx_bytes, &tx0.account_keys[0].as_ref()), Some(PUB_KEY_OFFSET) ); let tx1 = deserialize(&tx_bytes).unwrap(); assert_eq!(tx0, tx1); - assert_eq!(tx1.userdata, vec![1, 2, 3]); + assert_eq!(tx1.instructions[0].userdata, vec![1, 2, 3]); - tx0.userdata = vec![1, 2, 4]; + tx0.instructions[0].userdata = vec![1, 2, 4]; let sign_data0b = tx0.get_sign_data(); assert_ne!(sign_data0a, sign_data0b); } diff --git a/src/thin_client.rs b/src/thin_client.rs index 78708f429a..fdbda84170 100644 --- a/src/thin_client.rs +++ b/src/thin_client.rs @@ -540,11 +540,11 @@ mod tests { let last_id = client.get_last_id(); let mut tr2 = Transaction::system_new(&alice.keypair(), bob_pubkey, 501, last_id); - let mut instruction2 = deserialize(&tr2.userdata).unwrap(); + let mut instruction2 = deserialize(tr2.userdata(0)).unwrap(); if let SystemProgram::Move { ref mut tokens } = instruction2 { *tokens = 502; } - tr2.userdata = serialize(&instruction2).unwrap(); + tr2.instructions[0].userdata = serialize(&instruction2).unwrap(); let signature = client.transfer_signed(&tr2).unwrap(); client.poll_for_signature(&signature).unwrap(); diff --git a/src/tictactoe_program.rs b/src/tictactoe_program.rs index 31aec64843..b0f866c0a9 100644 --- a/src/tictactoe_program.rs +++ b/src/tictactoe_program.rs @@ -263,7 +263,11 @@ impl TicTacToeProgram { Pubkey::new(&TICTACTOE_PROGRAM_ID) } - pub fn process_transaction(tx: &Transaction, accounts: &mut [Account]) -> Result<()> { + pub fn process_transaction( + tx: &Transaction, + pix: usize, + accounts: &mut [&mut Account], + ) -> Result<()> { // accounts[1] must always be the Tic-tac-toe game state account if accounts.len() < 2 || !Self::check_id(&accounts[1].program_id) { error!("accounts[1] is not assigned to the TICTACTOE_PROGRAM_ID"); @@ -276,7 +280,7 @@ impl TicTacToeProgram { let mut program_state = Self::deserialize(&accounts[1].userdata)?; - let command = serde_cbor::from_slice::(&tx.userdata).map_err(|err| { + let command = serde_cbor::from_slice::(tx.userdata(pix)).map_err(|err| { error!("{:?}", err); Error::InvalidUserdata })?; @@ -288,14 +292,13 @@ impl TicTacToeProgram { error!("accounts[0] is not assigned to the TICTACTOE_PROGRAM_ID"); return Err(Error::InvalidArguments); } - // player X public key is in keys[2] - if accounts.len() < 3 { + if tx.key(pix, 2).is_none() { Err(Error::InvalidArguments)?; } - program_state.dispatch_command(&command, &tx.keys[2])?; + program_state.dispatch_command(&command, tx.key(pix, 2).unwrap())?; } else { - program_state.dispatch_command(&command, &tx.keys[0])?; + program_state.dispatch_command(&command, tx.key(pix, 0).unwrap())?; } program_state.serialize(&mut accounts[1].userdata)?; Ok(()) diff --git a/src/transaction.rs b/src/transaction.rs index 64c7108367..cf43ed3229 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -10,7 +10,20 @@ pub const SIGNED_DATA_OFFSET: usize = size_of::(); pub const SIG_OFFSET: usize = 0; pub const PUB_KEY_OFFSET: usize = size_of::() + size_of::(); -/// An instruction signed by a client with `Pubkey`. +/// An instruction to execute a program under `program_id` with the +/// specified accounts and userdata +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] +pub struct Instruction { + /// The program code that executes this transaction is identified by the program_id. + /// this is an offset into the Transaction::program_keys field + pub program_id: u8, + /// Indices into the keys array of which accounts to load + pub accounts: Vec, + /// Userdata to be stored in the account + pub userdata: Vec, +} + +/// An atomic transaction #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct Transaction { /// A digital signature of `keys`, `program_id`, `last_id`, `fee` and `userdata`, signed by `Pubkey`. @@ -18,13 +31,10 @@ pub struct Transaction { /// The `Pubkeys` that are executing this transaction userdata. The meaning of each key is /// program-specific. - /// * keys[0] - Typically this is the `caller` public key. `signature` is verified with keys[0]. + /// * account_keys[0] - Typically this is the `caller` public key. `signature` is verified with account_keys[0]. /// In the future which key pays the fee and which keys have signatures would be configurable. - /// * keys[1] - Typically this is the program context or the recipient of the tokens - pub keys: Vec, - - /// The program code that executes this transaction is identified by the program_id. - pub program_id: Pubkey, + /// * account_keys[1] - Typically this is the program context or the recipient of the tokens + pub account_keys: Vec, /// The ID of a recent ledger entry. pub last_id: Hash, @@ -32,18 +42,14 @@ pub struct Transaction { /// The number of tokens paid for processing and storage of this transaction. pub fee: i64, - /// Userdata to be stored in the account - pub userdata: Vec, + /// Keys indentifying programs in the instructions vector. + pub program_keys: Vec, + /// Programs that will be executed in sequence and commited in one atomic transaction if all + /// succeed. + pub instructions: Vec, } impl Transaction { - /// Create a signed transaction from the given `Instruction`. - /// * `from_keypair` - The key used to sign the transaction. This key is stored as keys[0] - /// * `transaction_keys` - The keys for the transaction. These are the program state - /// instances or token recipient keys. - /// * `userdata` - The input data that the program will execute with - /// * `last_id` - The PoH hash. - /// * `fee` - The transaction fee. pub fn new( from_keypair: &Keypair, transaction_keys: &[Pubkey], @@ -52,36 +58,80 @@ impl Transaction { last_id: Hash, fee: i64, ) -> Self { - let from = from_keypair.pubkey(); - let mut keys = vec![from]; - keys.extend_from_slice(transaction_keys); - let mut tx = Transaction { - signature: Signature::default(), - keys, - program_id, + let program_keys = vec![program_id]; + let instructions = vec![Instruction { + program_id: 0, + userdata, + accounts: (0..(transaction_keys.len() as u8 + 1)) + .into_iter() + .collect(), + }]; + Self::new_with_instructions( + from_keypair, + transaction_keys, last_id, fee, - userdata, + program_keys, + instructions, + ) + } + /// Create a signed transaction + /// * `from_keypair` - The key used to sign the transaction. This key is stored as keys[0] + /// * `account_keys` - The keys for the transaction. These are the program state + /// instances or token recipient keys. + /// * `last_id` - The PoH hash. + /// * `fee` - The transaction fee. + /// * `program_keys` - The keys that identify programs used in the `instruction` vector. + /// * `instructions` - The programs and their arguments that the transaction will execute atomically + pub fn new_with_instructions( + from_keypair: &Keypair, + keys: &[Pubkey], + last_id: Hash, + fee: i64, + program_keys: Vec, + instructions: Vec, + ) -> Self { + let from = from_keypair.pubkey(); + let mut account_keys = vec![from]; + account_keys.extend_from_slice(keys); + let mut tx = Transaction { + signature: Signature::default(), + account_keys, + last_id, + fee, + program_keys, + instructions, }; tx.sign(from_keypair); tx } - + pub fn userdata(&self, program_index: usize) -> &[u8] { + &self.instructions[program_index].userdata + } + pub fn key(&self, program_index: usize, kix: usize) -> Option<&Pubkey> { + self.instructions + .get(program_index) + .and_then(|p| p.accounts.get(kix)) + .and_then(|ai| self.account_keys.get(*ai as usize)) + } + pub fn program_id(&self, program_index: usize) -> &Pubkey { + &self.program_keys[self.instructions[program_index].program_id as usize] + } /// Get the transaction data to sign. pub fn get_sign_data(&self) -> Vec { - let mut data = serialize(&(&self.keys)).expect("serialize keys"); + let mut data = serialize(&self.account_keys).expect("serialize account_keys"); - let program_id = serialize(&(&self.program_id)).expect("serialize program_id"); - data.extend_from_slice(&program_id); - - let last_id_data = serialize(&(&self.last_id)).expect("serialize last_id"); + let last_id_data = serialize(&self.last_id).expect("serialize last_id"); data.extend_from_slice(&last_id_data); - let fee_data = serialize(&(&self.fee)).expect("serialize last_id"); + let fee_data = serialize(&self.fee).expect("serialize last_id"); data.extend_from_slice(&fee_data); - let userdata = serialize(&(&self.userdata)).expect("serialize userdata"); - data.extend_from_slice(&userdata); + let program_keys = serialize(&self.program_keys).expect("serialize program_keys"); + data.extend_from_slice(&program_keys); + + let instructions = serialize(&self.instructions).expect("serialize instructions"); + data.extend_from_slice(&instructions); data } @@ -98,8 +148,23 @@ impl Transaction { .verify(&self.from().as_ref(), &self.get_sign_data()) } - pub fn from(&self) -> &Pubkey { - &self.keys[0] + /// Verify that references in the instructions are valid + pub fn verify_refs(&self) -> bool { + for instruction in &self.instructions { + if (instruction.program_id as usize) >= self.program_keys.len() { + return false; + } + for account_index in &instruction.accounts { + if (*account_index as usize) >= self.account_keys.len() { + return false; + } + } + } + true + } + + fn from(&self) -> &Pubkey { + &self.account_keys[0] } // a hash of a slice of transactions only needs to hash the signatures @@ -118,6 +183,81 @@ mod tests { use bincode::serialize; use signature::GenKeys; + #[test] + fn test_refs() { + let key = Keypair::new(); + let key1 = Keypair::new().pubkey(); + let key2 = Keypair::new().pubkey(); + let prog1 = Keypair::new().pubkey(); + let prog2 = Keypair::new().pubkey(); + let instructions = vec![ + Instruction { + program_id: 0, + userdata: vec![], + accounts: vec![0, 1], + }, + Instruction { + program_id: 1, + userdata: vec![], + accounts: vec![0, 2], + }, + ]; + let tx = Transaction::new_with_instructions( + &key, + &[key1, key2], + Default::default(), + 0, + vec![prog1, prog2], + instructions, + ); + assert!(tx.verify_refs()); + assert_eq!(tx.key(0, 0), Some(&key.pubkey())); + assert_eq!(tx.key(1, 0), Some(&key.pubkey())); + assert_eq!(tx.key(0, 1), Some(&key1)); + assert_eq!(tx.key(1, 1), Some(&key2)); + assert_eq!(tx.key(2, 0), None); + assert_eq!(tx.key(0, 2), None); + assert_eq!(*tx.program_id(0), prog1); + assert_eq!(*tx.program_id(1), prog2); + } + #[test] + fn test_refs_invalid_program_id() { + let key = Keypair::new(); + let instructions = vec![Instruction { + program_id: 1, + userdata: vec![], + accounts: vec![], + }]; + let tx = Transaction::new_with_instructions( + &key, + &[], + Default::default(), + 0, + vec![], + instructions, + ); + assert!(!tx.verify_refs()); + } + #[test] + fn test_refs_invalid_account() { + let key = Keypair::new(); + let instructions = vec![Instruction { + program_id: 0, + userdata: vec![], + accounts: vec![1], + }]; + let tx = Transaction::new_with_instructions( + &key, + &[], + Default::default(), + 0, + vec![Default::default()], + instructions, + ); + assert_eq!(*tx.program_id(0), Default::default()); + assert!(!tx.verify_refs()); + } + /// Detect binary changes in the serialized contract userdata, which could have a downstream /// affect on SDKs and DApps #[test] @@ -144,18 +284,19 @@ mod tests { assert_eq!( serialize(&tx).unwrap(), vec![ - 88, 1, 212, 176, 31, 197, 35, 156, 135, 24, 30, 57, 204, 253, 224, 28, 89, 189, 53, - 64, 27, 148, 42, 199, 43, 236, 85, 182, 150, 64, 96, 53, 255, 235, 90, 197, 228, 6, - 105, 22, 140, 209, 206, 221, 85, 117, 125, 126, 11, 1, 176, 130, 57, 236, 7, 155, - 127, 58, 130, 92, 230, 219, 254, 0, 3, 0, 0, 0, 0, 0, 0, 0, 32, 253, 186, 201, 177, - 11, 117, 135, 187, 167, 181, 188, 22, 59, 206, 105, 231, 150, 215, 30, 78, 212, 76, - 16, 252, 180, 72, 134, 137, 247, 161, 68, 32, 253, 186, 201, 177, 11, 117, 135, - 187, 167, 181, 188, 22, 59, 206, 105, 231, 150, 215, 30, 78, 212, 76, 16, 252, 180, - 72, 134, 137, 247, 161, 68, 1, 1, 1, 4, 5, 6, 7, 8, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, - 9, 9, 9, 9, 9, 9, 8, 7, 6, 5, 4, 1, 1, 1, 2, 2, 2, 4, 5, 6, 7, 8, 9, 1, 1, 1, 1, 1, - 1, 1, 1, 1, 1, 1, 1, 1, 1, 9, 8, 7, 6, 5, 4, 2, 2, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 99, 0, 0, 0, 0, - 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3 + 234, 139, 34, 5, 120, 28, 107, 203, 69, 25, 236, 200, 164, 1, 12, 47, 147, 53, 41, + 143, 23, 116, 230, 203, 59, 228, 153, 14, 22, 241, 103, 226, 186, 169, 181, 65, 49, + 215, 44, 2, 61, 214, 113, 216, 184, 206, 147, 104, 140, 225, 138, 21, 172, 135, + 211, 80, 103, 80, 216, 106, 249, 86, 194, 1, 3, 0, 0, 0, 0, 0, 0, 0, 32, 253, 186, + 201, 177, 11, 117, 135, 187, 167, 181, 188, 22, 59, 206, 105, 231, 150, 215, 30, + 78, 212, 76, 16, 252, 180, 72, 134, 137, 247, 161, 68, 32, 253, 186, 201, 177, 11, + 117, 135, 187, 167, 181, 188, 22, 59, 206, 105, 231, 150, 215, 30, 78, 212, 76, 16, + 252, 180, 72, 134, 137, 247, 161, 68, 1, 1, 1, 4, 5, 6, 7, 8, 9, 9, 9, 9, 9, 9, 9, + 9, 9, 9, 9, 9, 9, 9, 9, 9, 8, 7, 6, 5, 4, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 99, 0, 0, 0, 0, 0, + 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2, 2, 2, 4, 5, 6, 7, 8, 9, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 9, 8, 7, 6, 5, 4, 2, 2, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, + 0, 0, 0, 0, 1, 2, 3, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3 ], ); } diff --git a/tests/programs.rs b/tests/programs.rs index e968f1bea0..9017e9e1f9 100644 --- a/tests/programs.rs +++ b/tests/programs.rs @@ -141,6 +141,14 @@ fn test_native_move_funds_succes_many_threads() { thread.join().unwrap(); } } +fn process_transaction( + tx: &Transaction, + accounts: &mut [Account], + loaded_programs: &RwLock>, +) { + let mut refs: Vec<&mut Account> = accounts.iter_mut().collect(); + SystemProgram::process_transaction(&tx, 0, &mut refs[..], loaded_programs) +} #[test] fn test_system_program_load_call() { @@ -158,7 +166,7 @@ fn test_system_program_load_call() { "move_funds".to_string(), ); - SystemProgram::process_transaction(&tx, &mut accounts, &loaded_programs); + process_transaction(&tx, &mut accounts, &loaded_programs); } // then call the program { @@ -211,7 +219,7 @@ fn test_system_program_load_call_many_threads() { "move_funds".to_string(), ); - SystemProgram::process_transaction(&tx, &mut accounts, &loaded_programs); + process_transaction(&tx, &mut accounts, &loaded_programs); } // then call the program {