address review comments

This commit is contained in:
Pankaj Garg 2019-02-19 17:11:43 -08:00 committed by Greg Fitzgerald
parent 2e75ff27ac
commit a33921ed34
5 changed files with 47 additions and 34 deletions

View File

@ -130,7 +130,7 @@ fn main() {
last_id = entry.id; last_id = entry.id;
num_entries += 1; num_entries += 1;
if let Err(e) = blocktree_processor::process_entry(&bank, &entry) { if let Err(e) = blocktree_processor::process_entry(&bank, &entry).0 {
eprintln!("verify failed at entry[{}], err: {:?}", i + 2, e); eprintln!("verify failed at entry[{}], err: {:?}", i + 2, e);
if !matches.is_present("continue") { if !matches.is_present("continue") {
exit(1); exit(1);

View File

@ -112,21 +112,18 @@ impl AccountsDB {
txs: &[Transaction], txs: &[Transaction],
res: &[Result<()>], res: &[Result<()>],
loaded: &[Result<(InstructionAccounts, InstructionLoaders)>], loaded: &[Result<(InstructionAccounts, InstructionLoaders)>],
) -> u64 { ) {
let mut fee = 0;
for (i, raccs) in loaded.iter().enumerate() { for (i, raccs) in loaded.iter().enumerate() {
if res[i].is_err() || raccs.is_err() { if res[i].is_err() || raccs.is_err() {
continue; continue;
} }
let tx = &txs[i]; let tx = &txs[i];
fee += tx.fee;
let acc = raccs.as_ref().unwrap(); let acc = raccs.as_ref().unwrap();
for (key, account) in tx.account_keys.iter().zip(acc.0.iter()) { for (key, account) in tx.account_keys.iter().zip(acc.0.iter()) {
self.store(purge, key, account); self.store(purge, key, account);
} }
} }
fee
} }
fn load_tx_accounts<U>( fn load_tx_accounts<U>(
checkpoints: &[U], checkpoints: &[U],
@ -375,7 +372,7 @@ impl Accounts {
txs: &[Transaction], txs: &[Transaction],
res: &[Result<()>], res: &[Result<()>],
loaded: &[Result<(InstructionAccounts, InstructionLoaders)>], loaded: &[Result<(InstructionAccounts, InstructionLoaders)>],
) -> u64 { ) {
self.accounts_db self.accounts_db
.write() .write()
.unwrap() .unwrap()

View File

@ -446,10 +446,9 @@ impl Bank {
txs: &[Transaction], txs: &[Transaction],
loaded_accounts: &[Result<(InstructionAccounts, InstructionLoaders)>], loaded_accounts: &[Result<(InstructionAccounts, InstructionLoaders)>],
executed: &[Result<()>], executed: &[Result<()>],
) -> u64 { ) {
let now = Instant::now(); let now = Instant::now();
let fee = self self.accounts
.accounts
.store_accounts(true, txs, executed, loaded_accounts); .store_accounts(true, txs, executed, loaded_accounts);
// once committed there is no way to unroll // once committed there is no way to unroll
@ -460,7 +459,6 @@ impl Bank {
txs.len(), txs.len(),
); );
self.update_transaction_statuses(txs, &executed); self.update_transaction_statuses(txs, &executed);
fee
} }
/// Process a batch of transactions. /// Process a batch of transactions.
@ -470,19 +468,18 @@ impl Bank {
txs: &[Transaction], txs: &[Transaction],
lock_results: Vec<Result<()>>, lock_results: Vec<Result<()>>,
max_age: usize, max_age: usize,
) -> (Vec<Result<()>>, u64) { ) -> Vec<Result<()>> {
let (loaded_accounts, executed) = let (loaded_accounts, executed) =
self.load_and_execute_transactions(txs, lock_results, max_age); self.load_and_execute_transactions(txs, lock_results, max_age);
let fee = self.commit_transactions(txs, &loaded_accounts, &executed); self.commit_transactions(txs, &loaded_accounts, &executed);
(executed, fee) executed
} }
#[must_use] #[must_use]
pub fn process_transactions(&self, txs: &[Transaction]) -> Vec<Result<()>> { pub fn process_transactions(&self, txs: &[Transaction]) -> Vec<Result<()>> {
let lock_results = self.lock_accounts(txs); let lock_results = self.lock_accounts(txs);
let (results, _fee) = let results = self.load_execute_and_commit_transactions(txs, lock_results, MAX_ENTRY_IDS);
self.load_execute_and_commit_transactions(txs, lock_results, MAX_ENTRY_IDS);
self.unlock_accounts(txs, &results); self.unlock_accounts(txs, &results);
results results
} }
@ -529,7 +526,7 @@ impl Bank {
parents parents
} }
pub fn collect_tx_fee(&self, pubkey: &Pubkey, fee: u64) { pub fn deposit(&self, pubkey: &Pubkey, fee: u64) {
if let Some(mut account) = self.get_account(pubkey) { if let Some(mut account) = self.get_account(pubkey) {
account.tokens += fee; account.tokens += fee;
self.accounts.store_slow(false, pubkey, &account); self.accounts.store_slow(false, pubkey, &account);
@ -898,7 +895,7 @@ mod tests {
let pay_alice = vec![tx1]; let pay_alice = vec![tx1];
let lock_result = bank.lock_accounts(&pay_alice); let lock_result = bank.lock_accounts(&pay_alice);
let (results_alice, _fee) = let results_alice =
bank.load_execute_and_commit_transactions(&pay_alice, lock_result, MAX_ENTRY_IDS); bank.load_execute_and_commit_transactions(&pay_alice, lock_result, MAX_ENTRY_IDS);
assert_eq!(results_alice[0], Ok(())); assert_eq!(results_alice[0], Ok(()));

View File

@ -16,21 +16,21 @@ use std::time::Instant;
pub const VERIFY_BLOCK_SIZE: usize = 16; pub const VERIFY_BLOCK_SIZE: usize = 16;
pub fn process_entry(bank: &Bank, entry: &Entry) -> Result<()> { pub fn process_entry(bank: &Bank, entry: &Entry) -> (Result<()>, u64) {
if !entry.is_tick() { if !entry.is_tick() {
for result in bank.process_transactions(&entry.transactions) { let old_results = bank.process_transactions(&entry.transactions);
match result { let results = ignore_program_errors(old_results);
// Entries that result in a ProgramError are still valid and are written in the let fee = entry
// ledger so map them to an ok return value .transactions
Err(BankError::ProgramError(_, _)) => Ok(()), .iter()
_ => result, .zip(&results)
}?; .map(|(tx, res)| if res.is_ok() { tx.fee } else { 0 })
} .sum();
(first_err(&results), fee)
} else { } else {
bank.register_tick(&entry.id); bank.register_tick(&entry.id);
(Ok(()), 0)
} }
Ok(())
} }
fn first_err(results: &[Result<()>]) -> Result<()> { fn first_err(results: &[Result<()>]) -> Result<()> {
@ -61,21 +61,25 @@ fn par_execute_entries(bank: &Bank, entries: &[(&Entry, Vec<Result<()>>)]) -> (R
let results_fees: Vec<(Result<()>, u64)> = entries let results_fees: Vec<(Result<()>, u64)> = entries
.into_par_iter() .into_par_iter()
.map(|(e, lock_results)| { .map(|(e, lock_results)| {
let (old_results, fee) = bank.load_execute_and_commit_transactions( let old_results = bank.load_execute_and_commit_transactions(
&e.transactions, &e.transactions,
lock_results.to_vec(), lock_results.to_vec(),
MAX_ENTRY_IDS, MAX_ENTRY_IDS,
); );
let results = ignore_program_errors(old_results); let results = ignore_program_errors(old_results);
bank.unlock_accounts(&e.transactions, &results); bank.unlock_accounts(&e.transactions, &results);
let fee = e
.transactions
.iter()
.zip(&results)
.map(|(tx, res)| if res.is_ok() { tx.fee } else { 0 })
.sum();
(first_err(&results), fee) (first_err(&results), fee)
}) })
.collect(); .collect();
let fee = results_fees.iter().map(|(_, fee)| fee).sum(); let fee = results_fees.iter().map(|(_, fee)| fee).sum();
let mut results = Vec::new(); let results: Vec<Result<()>> = results_fees.into_iter().map(|(res, _)| res).collect();
results.reserve_exact(results_fees.len());
results = results_fees.into_iter().map(|(res, _)| res).collect();
(first_err(&results[..]), fee) (first_err(&results[..]), fee)
} }
@ -150,11 +154,26 @@ fn process_block(
leader_scheduler: &Arc<RwLock<LeaderScheduler>>, leader_scheduler: &Arc<RwLock<LeaderScheduler>>,
) -> Result<()> { ) -> Result<()> {
for entry in entries { for entry in entries {
process_entry(bank, entry)?; let (res, fee) = process_entry(bank, entry);
let num_ticks = bank.tick_height();
let slot_num = leader_scheduler
.read()
.unwrap()
.tick_height_to_slot(num_ticks);
if let Some(leader) = leader_scheduler
.read()
.unwrap()
.get_leader_for_slot(slot_num)
{
// Credit the accumulated fees to the current leader and reset the fee to 0
bank.deposit(&leader, fee);
}
if entry.is_tick() { if entry.is_tick() {
let mut leader_scheduler = leader_scheduler.write().unwrap(); let mut leader_scheduler = leader_scheduler.write().unwrap();
leader_scheduler.update_tick_height(bank.tick_height(), bank); leader_scheduler.update_tick_height(bank.tick_height(), bank);
} }
res?;
} }
Ok(()) Ok(())

View File

@ -137,7 +137,7 @@ impl ReplayStage {
.get_leader_for_slot(slot_num) .get_leader_for_slot(slot_num)
{ {
// Credit the accumulated fees to the current leader and reset the fee to 0 // Credit the accumulated fees to the current leader and reset the fee to 0
bank.collect_tx_fee(&leader, *fees); bank.deposit(&leader, *fees);
*fees = 0; *fees = 0;
} }
if let Some(voting_keypair) = voting_keypair { if let Some(voting_keypair) = voting_keypair {