diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 860d85d8be..4c65e4e3fe 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -151,50 +151,68 @@ impl Accounts { // There is no way to predict what program will execute without an error // If a fee can pay for execution then the program will be scheduled - let mut accounts: TransactionAccounts = Vec::with_capacity(message.account_keys.len()); + let mut payer_index = None; let mut tx_rent: TransactionRent = 0; - for (i, key) in message + let mut accounts: Vec<_> = message .account_keys .iter() .enumerate() - .filter(|(i, key)| Self::is_non_loader_key(message, key, *i)) - { - let (account, rent) = AccountsDB::load(storage, ancestors, accounts_index, key) - .and_then(|(mut account, _)| { - if message.is_writable(i) && !account.executable { - let rent_due = rent_collector.update(&key, &mut account); - Some((account, rent_due)) - } else { - Some((account, 0)) + .map(|(i, key)| { + if Self::is_non_loader_key(message, key, i) { + if payer_index.is_none() { + payer_index = Some(i); } - }) - .unwrap_or_default(); + let (account, rent) = + AccountsDB::load(storage, ancestors, accounts_index, key) + .and_then(|(mut account, _)| { + if message.is_writable(i) && !account.executable { + let rent_due = rent_collector.update(&key, &mut account); + Some((account, rent_due)) + } else { + Some((account, 0)) + } + }) + .unwrap_or_default(); - accounts.push(account); - tx_rent += rent; - } + tx_rent += rent; + account + } else { + // Fill in an empty account for the program slots. + Account::default() + } + }) + .collect(); - if accounts.is_empty() || accounts[0].lamports == 0 { + if let Some(payer_index) = payer_index { + if payer_index != 0 { + warn!("Payer index should be 0! {:?}", tx); + } + if accounts[payer_index].lamports == 0 { + error_counters.account_not_found += 1; + Err(TransactionError::AccountNotFound) + } else { + let min_balance = match get_system_account_kind(&accounts[payer_index]) + .ok_or_else(|| { + error_counters.invalid_account_for_fee += 1; + TransactionError::InvalidAccountForFee + })? { + SystemAccountKind::System => 0, + SystemAccountKind::Nonce => { + rent_collector.rent.minimum_balance(nonce::State::size()) + } + }; + + if accounts[payer_index].lamports < fee + min_balance { + error_counters.insufficient_funds += 1; + Err(TransactionError::InsufficientFundsForFee) + } else { + accounts[payer_index].lamports -= fee; + Ok((accounts, tx_rent)) + } + } + } else { error_counters.account_not_found += 1; Err(TransactionError::AccountNotFound) - } else { - let min_balance = match get_system_account_kind(&accounts[0]).ok_or_else(|| { - error_counters.invalid_account_for_fee += 1; - TransactionError::InvalidAccountForFee - })? { - SystemAccountKind::System => 0, - SystemAccountKind::Nonce => { - rent_collector.rent.minimum_balance(nonce::State::size()) - } - }; - - if accounts[0].lamports < fee + min_balance { - error_counters.insufficient_funds += 1; - Err(TransactionError::InsufficientFundsForFee) - } else { - accounts[0].lamports -= fee; - Ok((accounts, tx_rent)) - } } } } @@ -651,8 +669,8 @@ impl Accounts { .account_keys .iter() .enumerate() - .filter(|(i, key)| Self::is_non_loader_key(message, key, *i)) .zip(acc.0.iter_mut()) + .filter(|((i, key), _account)| Self::is_non_loader_key(message, key, *i)) { nonce_utils::prepare_if_nonce_account( account, @@ -1050,7 +1068,7 @@ mod tests { Ok((transaction_accounts, transaction_loaders, _transaction_rents)), _hash_age_kind, ) => { - assert_eq!(transaction_accounts.len(), 2); + assert_eq!(transaction_accounts.len(), 3); assert_eq!(transaction_accounts[0], accounts[0].1); assert_eq!(transaction_loaders.len(), 1); assert_eq!(transaction_loaders[0].len(), 0); @@ -1288,7 +1306,7 @@ mod tests { Ok((transaction_accounts, transaction_loaders, _transaction_rents)), _hash_age_kind, ) => { - assert_eq!(transaction_accounts.len(), 1); + assert_eq!(transaction_accounts.len(), 3); assert_eq!(transaction_accounts[0], accounts[0].1); assert_eq!(transaction_loaders.len(), 2); assert_eq!(transaction_loaders[0].len(), 1); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 407a9047e0..7a20104020 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4603,7 +4603,10 @@ mod tests { bank.last_blockhash(), ); - assert_eq!(bank.process_transaction(&tx), Ok(())); + assert_eq!( + bank.process_transaction(&tx), + Err(TransactionError::InvalidAccountIndex) + ); assert_eq!(bank.get_balance(&key.pubkey()), 0); } @@ -5840,15 +5843,11 @@ mod tests { tx.message.account_keys.push(Pubkey::new_rand()); - fn mock_vote_processor( - _pubkey: &Pubkey, - _ka: &[KeyedAccount], - _data: &[u8], - ) -> std::result::Result<(), InstructionError> { - Ok(()) - } - - bank.add_static_program("mock_vote", solana_vote_program::id(), mock_vote_processor); + bank.add_static_program( + "mock_vote", + solana_vote_program::id(), + mock_ok_vote_processor, + ); let result = bank.process_transaction(&tx); assert_eq!(result, Ok(())); let account = bank.get_account(&solana_vote_program::id()).unwrap(); @@ -5883,4 +5882,96 @@ mod tests { // capitalization assert_eq!(bank.capitalization(), pre_capitalization - burn_amount); } + + #[test] + fn test_program_id_as_payer() { + solana_logger::setup(); + let (genesis_config, mint_keypair) = create_genesis_config(500); + let mut bank = Bank::new(&genesis_config); + + let from_pubkey = Pubkey::new_rand(); + let to_pubkey = Pubkey::new_rand(); + + let account_metas = vec![ + AccountMeta::new(from_pubkey, false), + AccountMeta::new(to_pubkey, false), + ]; + + bank.add_static_program( + "mock_vote", + solana_vote_program::id(), + mock_ok_vote_processor, + ); + + let instruction = Instruction::new(solana_vote_program::id(), &10, account_metas); + let mut tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + + info!( + "mint: {} account keys: {:?}", + mint_keypair.pubkey(), + tx.message.account_keys + ); + assert_eq!(tx.message.account_keys.len(), 4); + tx.message.account_keys.clear(); + tx.message.account_keys.push(solana_vote_program::id()); + tx.message.account_keys.push(mint_keypair.pubkey()); + tx.message.account_keys.push(from_pubkey); + tx.message.account_keys.push(to_pubkey); + tx.message.instructions[0].program_id_index = 0; + tx.message.instructions[0].accounts.clear(); + tx.message.instructions[0].accounts.push(2); + tx.message.instructions[0].accounts.push(3); + + let result = bank.process_transaction(&tx); + assert_eq!(result, Err(TransactionError::InvalidAccountIndex)); + } + + fn mock_ok_vote_processor( + _pubkey: &Pubkey, + _ka: &[KeyedAccount], + _data: &[u8], + ) -> std::result::Result<(), InstructionError> { + Ok(()) + } + + #[test] + fn test_ref_account_key_after_program_id() { + let (genesis_config, mint_keypair) = create_genesis_config(500); + let mut bank = Bank::new(&genesis_config); + + let from_pubkey = Pubkey::new_rand(); + let to_pubkey = Pubkey::new_rand(); + + let account_metas = vec![ + AccountMeta::new(from_pubkey, false), + AccountMeta::new(to_pubkey, false), + ]; + + bank.add_static_program( + "mock_vote", + solana_vote_program::id(), + mock_ok_vote_processor, + ); + + let instruction = Instruction::new(solana_vote_program::id(), &10, account_metas); + let mut tx = Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + bank.last_blockhash(), + ); + + tx.message.account_keys.push(Pubkey::new_rand()); + assert_eq!(tx.message.account_keys.len(), 5); + tx.message.instructions[0].accounts.remove(0); + tx.message.instructions[0].accounts.push(4); + + let result = bank.process_transaction(&tx); + assert_eq!(result, Ok(())); + } } diff --git a/runtime/tests/stake.rs b/runtime/tests/stake.rs index 66cbcba2bf..c6c63355d3 100644 --- a/runtime/tests/stake.rs +++ b/runtime/tests/stake.rs @@ -137,14 +137,17 @@ fn test_stake_create_and_split_single_signature() { Pubkey::create_with_seed(&staker_pubkey, "split_stake", &solana_stake_program::id()) .unwrap(); // Test split - let message = Message::new(&stake_instruction::split_with_seed( - &stake_address, // original - &staker_pubkey, // authorized - lamports / 2, - &split_stake_address, // new address - &staker_pubkey, // base - "split_stake", // seed - )); + let message = Message::new_with_payer( + &stake_instruction::split_with_seed( + &stake_address, // original + &staker_pubkey, // authorized + lamports / 2, + &split_stake_address, // new address + &staker_pubkey, // base + "split_stake", // seed + ), + Some(&staker_keypair.pubkey()), + ); assert!(bank_client .send_message(&[&staker_keypair], message) diff --git a/sdk/src/message.rs b/sdk/src/message.rs index 4bf59fd0ed..740285ef71 100644 --- a/sdk/src/message.rs +++ b/sdk/src/message.rs @@ -165,19 +165,27 @@ pub struct Message { impl Sanitize for Message { fn sanitize(&self) -> std::result::Result<(), SanitizeError> { - if self.header.num_required_signatures as usize > self.account_keys.len() { - return Err(SanitizeError::IndexOutOfBounds); - } - if self.header.num_readonly_unsigned_accounts as usize - + self.header.num_readonly_signed_accounts as usize + // signing area and read-only non-signing area should not overlap + if self.header.num_required_signatures as usize + + self.header.num_readonly_unsigned_accounts as usize > self.account_keys.len() { return Err(SanitizeError::IndexOutOfBounds); } + + // there should be at least 1 RW fee-payer account. + if self.header.num_readonly_signed_accounts >= self.header.num_required_signatures { + return Err(SanitizeError::IndexOutOfBounds); + } + for ci in &self.instructions { if ci.program_id_index as usize >= self.account_keys.len() { return Err(SanitizeError::IndexOutOfBounds); } + // A program cannot be a payer. + if ci.program_id_index == 0 { + return Err(SanitizeError::IndexOutOfBounds); + } for ai in &ci.accounts { if *ai as usize >= self.account_keys.len() { return Err(SanitizeError::IndexOutOfBounds); diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index 7ec66d60ef..4b3ab36621 100644 --- a/sdk/src/transaction.rs +++ b/sdk/src/transaction.rs @@ -510,6 +510,21 @@ mod tests { tx = o.clone(); tx.message.instructions[0].accounts[0] = 3; assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + + tx = o.clone(); + tx.message.instructions[0].program_id_index = 0; + assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + + tx = o.clone(); + tx.message.header.num_readonly_signed_accounts = 2; + tx.message.header.num_readonly_unsigned_accounts = 3; + tx.message.account_keys.resize(4, Pubkey::default()); + assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + + tx = o.clone(); + tx.message.header.num_readonly_signed_accounts = 2; + tx.message.header.num_required_signatures = 1; + assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); } fn create_sample_transaction() -> Transaction {