diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index cf89d5380c..6de3219aaf 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -20,7 +20,7 @@ use solana_runtime::bank::Bank; use solana_runtime::locked_accounts_results::LockedAccountsResults; use solana_sdk::pubkey::Pubkey; use solana_sdk::timing::{self, duration_as_us, DEFAULT_TICKS_PER_SLOT, MAX_RECENT_BLOCKHASHES}; -use solana_sdk::transaction::{self, Transaction, TransactionError}; +use solana_sdk::transaction::{self, Transaction}; use std::cmp; use std::net::UdpSocket; use std::sync::atomic::{AtomicBool, Ordering}; @@ -367,14 +367,10 @@ impl BankingStage { let processed_transactions: Vec<_> = results .iter() .zip(txs.iter()) - .filter_map(|(r, x)| match r { - Ok(_) => Some(x.clone()), - Err(TransactionError::InstructionError(index, err)) => { - debug!("instruction error {:?}, {:?}", index, err); + .filter_map(|(r, x)| { + if Bank::can_commit(r) { Some(x.clone()) - } - Err(ref e) => { - debug!("process transaction failed {:?}", e); + } else { None } }) @@ -667,6 +663,7 @@ mod tests { use solana_sdk::instruction::InstructionError; use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::system_transaction; + use solana_sdk::transaction::TransactionError; use std::sync::mpsc::channel; use std::thread::sleep; diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c301364bbf..9e07ecc8ca 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -423,31 +423,24 @@ impl Bank { self.status_cache.write().unwrap().clear_signatures(); } + pub fn can_commit(result: &Result<()>) -> bool { + match result { + Ok(_) => true, + Err(TransactionError::InstructionError(_, _)) => true, + Err(_) => false, + } + } + fn update_transaction_statuses(&self, txs: &[Transaction], res: &[Result<()>]) { let mut status_cache = self.status_cache.write().unwrap(); for (i, tx) in txs.iter().enumerate() { - match &res[i] { - Ok(_) => { - if !tx.signatures.is_empty() { - status_cache.insert( - &tx.message().recent_blockhash, - &tx.signatures[0], - self.slot(), - Ok(()), - ); - } - } - Err(TransactionError::InstructionError(b, e)) => { - if !tx.signatures.is_empty() { - status_cache.insert( - &tx.message().recent_blockhash, - &tx.signatures[0], - self.slot(), - Err(TransactionError::InstructionError(*b, e.clone())), - ); - } - } - Err(_) => (), + if Self::can_commit(&res[i]) && !tx.signatures.is_empty() { + status_cache.insert( + &tx.message().recent_blockhash, + &tx.signatures[0], + self.slot(), + res[i].clone(), + ); } } } @@ -769,7 +762,9 @@ impl Bank { warn!("=========== FIXME: commit_transactions() working on a frozen bank! ================"); } - self.is_delta.store(true, Ordering::Relaxed); + if executed.iter().any(|res| Self::can_commit(res)) { + self.is_delta.store(true, Ordering::Relaxed); + } // TODO: put this assert back in // assert!(!self.is_frozen()); @@ -1952,4 +1947,44 @@ mod tests { bank.tick_height.store(max_tick_height, Ordering::Relaxed); assert!(bank.is_votable()); } + + #[test] + fn test_is_delta_with_no_committables() { + let (genesis_block, mint_keypair) = GenesisBlock::new(8000); + let bank = Bank::new(&genesis_block); + bank.is_delta.store(false, Ordering::Relaxed); + + let keypair1 = Keypair::new(); + let keypair2 = Keypair::new(); + let fail_tx = system_transaction::create_user_account( + &keypair1, + &keypair2.pubkey(), + 1, + bank.last_blockhash(), + 0, + ); + + // Should fail with TransactionError::AccountNotFound, which means + // the account which this tx operated on will not be committed. Thus + // the bank is_delta should still be false + assert_eq!( + bank.process_transaction(&fail_tx), + Err(TransactionError::AccountNotFound) + ); + + // Check the bank is_delta is still false + assert!(!bank.is_delta.load(Ordering::Relaxed)); + + // Should fail with InstructionError, but InstructionErrors are committable, + // so is_delta should be true + assert_eq!( + bank.transfer(10_001, &mint_keypair, &Pubkey::new_rand()), + Err(TransactionError::InstructionError( + 0, + InstructionError::new_result_with_negative_lamports(), + )) + ); + + assert!(bank.is_delta.load(Ordering::Relaxed)); + } }