Relocate transaction reference verification to join the other validity checks
This commit is contained in:
		@@ -388,7 +388,7 @@ impl BankingStage {
 | 
				
			|||||||
                    .filter_map(|((tx, ver), index)| match tx {
 | 
					                    .filter_map(|((tx, ver), index)| match tx {
 | 
				
			||||||
                        None => None,
 | 
					                        None => None,
 | 
				
			||||||
                        Some(tx) => {
 | 
					                        Some(tx) => {
 | 
				
			||||||
                            if tx.verify_refs() && ver != 0 {
 | 
					                            if ver != 0 {
 | 
				
			||||||
                                Some((tx, index))
 | 
					                                Some((tx, index))
 | 
				
			||||||
                            } else {
 | 
					                            } else {
 | 
				
			||||||
                                None
 | 
					                                None
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -32,6 +32,7 @@ pub struct ErrorCounters {
 | 
				
			|||||||
    pub blockhash_too_old: usize,
 | 
					    pub blockhash_too_old: usize,
 | 
				
			||||||
    pub reserve_blockhash: usize,
 | 
					    pub reserve_blockhash: usize,
 | 
				
			||||||
    pub insufficient_funds: usize,
 | 
					    pub insufficient_funds: usize,
 | 
				
			||||||
 | 
					    pub invalid_account_index: usize,
 | 
				
			||||||
    pub duplicate_signature: usize,
 | 
					    pub duplicate_signature: usize,
 | 
				
			||||||
    pub call_chain_too_deep: usize,
 | 
					    pub call_chain_too_deep: usize,
 | 
				
			||||||
    pub missing_signature_for_fee: usize,
 | 
					    pub missing_signature_for_fee: usize,
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -467,6 +467,24 @@ impl Bank {
 | 
				
			|||||||
        self.accounts
 | 
					        self.accounts
 | 
				
			||||||
            .load_accounts(self.accounts_id, txs, results, error_counters)
 | 
					            .load_accounts(self.accounts_id, txs, results, error_counters)
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					    fn check_refs(
 | 
				
			||||||
 | 
					        &self,
 | 
				
			||||||
 | 
					        txs: &[Transaction],
 | 
				
			||||||
 | 
					        lock_results: Vec<Result<()>>,
 | 
				
			||||||
 | 
					        error_counters: &mut ErrorCounters,
 | 
				
			||||||
 | 
					    ) -> Vec<Result<()>> {
 | 
				
			||||||
 | 
					        txs.iter()
 | 
				
			||||||
 | 
					            .zip(lock_results.into_iter())
 | 
				
			||||||
 | 
					            .map(|(tx, lock_res)| {
 | 
				
			||||||
 | 
					                if lock_res.is_ok() && !tx.verify_refs() {
 | 
				
			||||||
 | 
					                    error_counters.invalid_account_index += 1;
 | 
				
			||||||
 | 
					                    Err(TransactionError::InvalidAccountIndex)
 | 
				
			||||||
 | 
					                } else {
 | 
				
			||||||
 | 
					                    lock_res
 | 
				
			||||||
 | 
					                }
 | 
				
			||||||
 | 
					            })
 | 
				
			||||||
 | 
					            .collect()
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
    fn check_age(
 | 
					    fn check_age(
 | 
				
			||||||
        &self,
 | 
					        &self,
 | 
				
			||||||
        txs: &[Transaction],
 | 
					        txs: &[Transaction],
 | 
				
			||||||
@@ -524,7 +542,8 @@ impl Bank {
 | 
				
			|||||||
        debug!("processing transactions: {}", txs.len());
 | 
					        debug!("processing transactions: {}", txs.len());
 | 
				
			||||||
        let mut error_counters = ErrorCounters::default();
 | 
					        let mut error_counters = ErrorCounters::default();
 | 
				
			||||||
        let now = Instant::now();
 | 
					        let now = Instant::now();
 | 
				
			||||||
        let age_results = self.check_age(txs, lock_results, max_age, &mut error_counters);
 | 
					        let refs_results = self.check_refs(txs, lock_results, &mut error_counters);
 | 
				
			||||||
 | 
					        let age_results = self.check_age(txs, refs_results, max_age, &mut error_counters);
 | 
				
			||||||
        let sig_results = self.check_signatures(txs, age_results, &mut error_counters);
 | 
					        let sig_results = self.check_signatures(txs, age_results, &mut error_counters);
 | 
				
			||||||
        let mut loaded_accounts = self.load_accounts(txs, sig_results, &mut error_counters);
 | 
					        let mut loaded_accounts = self.load_accounts(txs, sig_results, &mut error_counters);
 | 
				
			||||||
        let tick_height = self.tick_height();
 | 
					        let tick_height = self.tick_height();
 | 
				
			||||||
@@ -582,6 +601,12 @@ impl Bank {
 | 
				
			|||||||
                error_counters.blockhash_not_found
 | 
					                error_counters.blockhash_not_found
 | 
				
			||||||
            );
 | 
					            );
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
 | 
					        if 0 != error_counters.invalid_account_index {
 | 
				
			||||||
 | 
					            inc_new_counter_info!(
 | 
				
			||||||
 | 
					                "bank-process_transactions-error-invalid_account_index",
 | 
				
			||||||
 | 
					                error_counters.invalid_account_index
 | 
				
			||||||
 | 
					            );
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
        if 0 != error_counters.reserve_blockhash {
 | 
					        if 0 != error_counters.reserve_blockhash {
 | 
				
			||||||
            inc_new_counter_info!(
 | 
					            inc_new_counter_info!(
 | 
				
			||||||
                "bank-process_transactions-error-reserve_blockhash",
 | 
					                "bank-process_transactions-error-reserve_blockhash",
 | 
				
			||||||
@@ -1275,6 +1300,35 @@ mod tests {
 | 
				
			|||||||
            .is_ok());
 | 
					            .is_ok());
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    #[test]
 | 
				
			||||||
 | 
					    fn test_bank_invalid_account_index() {
 | 
				
			||||||
 | 
					        let (genesis_block, mint_keypair) = GenesisBlock::new(1);
 | 
				
			||||||
 | 
					        let keypair = Keypair::new();
 | 
				
			||||||
 | 
					        let bank = Bank::new(&genesis_block);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        let tx = SystemTransaction::new_move(
 | 
				
			||||||
 | 
					            &mint_keypair,
 | 
				
			||||||
 | 
					            &keypair.pubkey(),
 | 
				
			||||||
 | 
					            1,
 | 
				
			||||||
 | 
					            genesis_block.hash(),
 | 
				
			||||||
 | 
					            0,
 | 
				
			||||||
 | 
					        );
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        let mut tx_invalid_program_index = tx.clone();
 | 
				
			||||||
 | 
					        tx_invalid_program_index.instructions[0].program_ids_index = 42;
 | 
				
			||||||
 | 
					        assert_eq!(
 | 
				
			||||||
 | 
					            bank.process_transaction(&tx_invalid_program_index),
 | 
				
			||||||
 | 
					            Err(TransactionError::InvalidAccountIndex)
 | 
				
			||||||
 | 
					        );
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        let mut tx_invalid_account_index = tx.clone();
 | 
				
			||||||
 | 
					        tx_invalid_account_index.instructions[0].accounts[0] = 42;
 | 
				
			||||||
 | 
					        assert_eq!(
 | 
				
			||||||
 | 
					            bank.process_transaction(&tx_invalid_account_index),
 | 
				
			||||||
 | 
					            Err(TransactionError::InvalidAccountIndex)
 | 
				
			||||||
 | 
					        );
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    #[test]
 | 
					    #[test]
 | 
				
			||||||
    fn test_bank_pay_to_self() {
 | 
					    fn test_bank_pay_to_self() {
 | 
				
			||||||
        let (genesis_block, mint_keypair) = GenesisBlock::new(1);
 | 
					        let (genesis_block, mint_keypair) = GenesisBlock::new(1);
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -189,6 +189,9 @@ pub enum TransactionError {
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    /// Transaction has a fee but has no signature present
 | 
					    /// Transaction has a fee but has no signature present
 | 
				
			||||||
    MissingSignatureForFee,
 | 
					    MissingSignatureForFee,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    /// Transaction contains an invalid account reference
 | 
				
			||||||
 | 
					    InvalidAccountIndex,
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/// An atomic transaction
 | 
					/// An atomic transaction
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user