From 1809277e052f22a54616b57e18adc6bac8bf19a8 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Sat, 16 Feb 2019 07:28:58 -0700 Subject: [PATCH] Encapsulate Bank accounts That way we don't need to TODOs saying "don't forget to iterate over checkpoints too". It should be assumed that when the bank references its previous checkpoint all its methods would acknowledge it. --- src/bank.rs | 29 ++++++++++++++-- src/compute_leader_confirmation_service.rs | 20 +---------- src/leader_scheduler.rs | 40 ++++++++-------------- 3 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/bank.rs b/src/bank.rs index b85a65877..0d01dde46 100644 --- a/src/bank.rs +++ b/src/bank.rs @@ -32,7 +32,7 @@ use solana_sdk::system_transaction::SystemTransaction; use solana_sdk::timing::duration_as_us; use solana_sdk::token_program; use solana_sdk::transaction::Transaction; -use solana_sdk::vote_program; +use solana_sdk::vote_program::{self, VoteState}; use std; use std::result; use std::sync::{Arc, RwLock}; @@ -93,7 +93,7 @@ type BankStatusCache = StatusCache; /// Manager for the state of all accounts and programs after processing its entries. pub struct Bank { - pub accounts: Accounts, + accounts: Accounts, /// A cache of signature statuses status_cache: RwLock, @@ -183,7 +183,7 @@ impl Bank { executable: false, }; - let mut vote_state = vote_program::VoteState::new( + let mut vote_state = VoteState::new( genesis_block.bootstrap_leader_id, genesis_block.bootstrap_leader_id, ); @@ -807,6 +807,29 @@ impl Bank { } } + pub fn vote_states(&self, cond: F) -> Vec + where + F: Fn(&VoteState) -> bool, + { + self.accounts + .accounts_db + .read() + .unwrap() + .accounts + .values() + .filter_map(|account| { + if vote_program::check_id(&account.owner) { + if let Ok(vote_state) = VoteState::deserialize(&account.userdata) { + if cond(&vote_state) { + return Some(vote_state); + } + } + } + None + }) + .collect() + } + #[cfg(test)] fn get_current_leader(&self) -> Option { let tick_height = self.tick_height(); diff --git a/src/compute_leader_confirmation_service.rs b/src/compute_leader_confirmation_service.rs index 29ed8991c..9c037f0eb 100644 --- a/src/compute_leader_confirmation_service.rs +++ b/src/compute_leader_confirmation_service.rs @@ -8,7 +8,6 @@ use crate::service::Service; use solana_metrics::{influxdb, submit}; use solana_sdk::pubkey::Pubkey; use solana_sdk::timing; -use solana_sdk::vote_program::{self, VoteState}; use std::result; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -37,24 +36,7 @@ impl ComputeLeaderConfirmationService { // Hold an accounts_db read lock as briefly as possible, just long enough to collect all // the vote states - let vote_states: Vec = bank - .accounts - .accounts_db - .read() - .unwrap() - .accounts - .values() - .filter_map(|account| { - if vote_program::check_id(&account.owner) { - if let Ok(vote_state) = VoteState::deserialize(&account.userdata) { - if leader_id != vote_state.node_id { - return Some(vote_state); - } - } - } - None - }) - .collect(); + let vote_states = bank.vote_states(|vote_state| leader_id != vote_state.node_id); let mut ticks_and_stakes: Vec<(u64, u64)> = vote_states .iter() diff --git a/src/leader_scheduler.rs b/src/leader_scheduler.rs index a5c66d9b1..9ba1f924d 100644 --- a/src/leader_scheduler.rs +++ b/src/leader_scheduler.rs @@ -11,7 +11,7 @@ use solana_sdk::hash::{hash, Hash}; use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::system_transaction::SystemTransaction; -use solana_sdk::vote_program::{self, VoteState}; +use solana_sdk::vote_program::VoteState; use solana_sdk::vote_transaction::VoteTransaction; use std::io::Cursor; use std::sync::Arc; @@ -195,6 +195,15 @@ impl LeaderScheduler { } } + // Return true of the latest vote is between the lower and upper bounds (inclusive) + fn is_active_staker(vote_state: &VoteState, lower_bound: u64, upper_bound: u64) -> bool { + vote_state + .votes + .back() + .filter(|vote| vote.tick_height >= lower_bound && vote.tick_height <= upper_bound) + .is_some() + } + // TODO: We use a HashSet for now because a single validator could potentially register // multiple vote account. Once that is no longer possible (see the TODO in vote_program.rs, // process_transaction(), case VoteInstruction::RegisterAccount), we can use a vector. @@ -207,31 +216,10 @@ impl LeaderScheduler { upper_bound ); - { - let accounts = bank.accounts.accounts_db.read().unwrap(); - // TODO: iterate through checkpoints, too - accounts - .accounts - .values() - .filter_map(|account| { - if vote_program::check_id(&account.owner) { - if let Ok(vote_state) = VoteState::deserialize(&account.userdata) { - trace!("get_active_set: account vote_state: {:?}", vote_state); - return vote_state - .votes - .back() - .filter(|vote| { - vote.tick_height >= lower_bound - && vote.tick_height <= upper_bound - }) - .map(|_| vote_state.staker_id); - } - } - - None - }) - .collect() - } + bank.vote_states(|vote_state| Self::is_active_staker(vote_state, lower_bound, upper_bound)) + .iter() + .map(|vote_state| vote_state.staker_id) + .collect() } // Updates the leader schedule to include ticks from tick_height to the first tick of the next epoch