From ebadbce920383ad8817765fda91cd164ae39e0fb Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Wed, 8 Jul 2020 18:50:13 -0600 Subject: [PATCH] Drop bank from BlockCommitmentCache (#10959) * Remove bank reference from BlockCommitmentCache * Don't use a Bank to create BlockCommitmentCache * Rename recent_slot to slot --- core/src/commitment_service.rs | 2 +- core/src/rpc.rs | 8 ++++---- core/src/rpc_pubsub.rs | 5 ++--- core/src/rpc_subscriptions.rs | 12 +++++------ runtime/src/commitment.rs | 37 ++++++++++++++-------------------- 5 files changed, 27 insertions(+), 37 deletions(-) diff --git a/core/src/commitment_service.rs b/core/src/commitment_service.rs index fb6bd97407..9974faafde 100644 --- a/core/src/commitment_service.rs +++ b/core/src/commitment_service.rs @@ -116,7 +116,7 @@ impl AggregateCommitmentService { block_commitment, highest_confirmed_root, aggregation_data.total_stake, - aggregation_data.bank, + aggregation_data.bank.slot(), aggregation_data.root, aggregation_data.root, ); diff --git a/core/src/rpc.rs b/core/src/rpc.rs index f70da67ee3..e9357d0f07 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -183,7 +183,7 @@ impl JsonRpcRequestProcessor { HashMap::new(), 0, 0, - bank.clone(), + bank.slot(), 0, 0, ))), @@ -1798,7 +1798,7 @@ pub mod tests { block_commitment, 0, 10, - bank.clone(), + bank.slot(), 0, 0, ))); @@ -3326,7 +3326,7 @@ pub mod tests { block_commitment, 0, 42, - bank_forks.read().unwrap().working_bank(), + bank_forks.read().unwrap().highest_slot(), 0, 0, ))); @@ -3830,7 +3830,7 @@ pub mod tests { block_commitment, highest_confirmed_root, 50, - bank.clone(), + bank.slot(), 0, 0, ); diff --git a/core/src/rpc_pubsub.rs b/core/src/rpc_pubsub.rs index f5d7339f02..a996bcfd12 100644 --- a/core/src/rpc_pubsub.rs +++ b/core/src/rpc_pubsub.rs @@ -525,9 +525,8 @@ mod tests { subscriptions: Arc::new(RpcSubscriptions::new( &Arc::new(AtomicBool::new(false)), bank_forks.clone(), - Arc::new(RwLock::new(BlockCommitmentCache::new_for_tests_with_bank( - bank_forks.read().unwrap().get(1).unwrap().clone(), - 1, + Arc::new(RwLock::new(BlockCommitmentCache::new_for_tests_with_slots( + 1, 1, ))), )), uid: Arc::new(atomic::AtomicUsize::default()), diff --git a/core/src/rpc_subscriptions.rs b/core/src/rpc_subscriptions.rs index abd40189b9..88f0c85f16 100644 --- a/core/src/rpc_subscriptions.rs +++ b/core/src/rpc_subscriptions.rs @@ -961,9 +961,8 @@ pub(crate) mod tests { let subscriptions = RpcSubscriptions::new( &exit, bank_forks.clone(), - Arc::new(RwLock::new(BlockCommitmentCache::new_for_tests_with_bank( - bank_forks.read().unwrap().get(1).unwrap().clone(), - 1, + Arc::new(RwLock::new(BlockCommitmentCache::new_for_tests_with_slots( + 1, 1, ))), ); subscriptions.add_account_subscription( @@ -1158,7 +1157,7 @@ pub(crate) mod tests { block_commitment.entry(0).or_insert(cache0); block_commitment.entry(1).or_insert(cache1); let block_commitment_cache = - BlockCommitmentCache::new(block_commitment, 0, 10, bank1, 0, 0); + BlockCommitmentCache::new(block_commitment, 0, 10, bank1.slot(), 0, 0); let exit = Arc::new(AtomicBool::new(false)); let subscriptions = RpcSubscriptions::new( @@ -1433,9 +1432,8 @@ pub(crate) mod tests { let subscriptions = RpcSubscriptions::new( &exit, bank_forks.clone(), - Arc::new(RwLock::new(BlockCommitmentCache::new_for_tests_with_bank( - bank_forks.read().unwrap().get(1).unwrap().clone(), - 1, + Arc::new(RwLock::new(BlockCommitmentCache::new_for_tests_with_slots( + 1, 1, ))), ); let sub_id0 = SubscriptionId::Number(0 as u64); diff --git a/runtime/src/commitment.rs b/runtime/src/commitment.rs index 481620c912..eec37e187a 100644 --- a/runtime/src/commitment.rs +++ b/runtime/src/commitment.rs @@ -1,7 +1,6 @@ -use crate::bank::Bank; use solana_sdk::clock::Slot; use solana_vote_program::vote_state::MAX_LOCKOUT_HISTORY; -use std::{collections::HashMap, sync::Arc}; +use std::collections::HashMap; pub const VOTE_THRESHOLD_SIZE: f64 = 2f64 / 3f64; @@ -41,7 +40,8 @@ pub struct BlockCommitmentCache { block_commitment: HashMap, highest_confirmed_root: Slot, total_stake: u64, - bank: Arc, + /// The slot of the bank from which all other slots were calculated. + slot: Slot, root: Slot, pub highest_confirmed_slot: Slot, } @@ -53,7 +53,7 @@ impl std::fmt::Debug for BlockCommitmentCache { .field("total_stake", &self.total_stake) .field( "bank", - &format_args!("Bank({{current_slot: {:?}}})", self.bank.slot()), + &format_args!("Bank({{current_slot: {:?}}})", self.slot), ) .field("root", &self.root) .finish() @@ -65,7 +65,7 @@ impl BlockCommitmentCache { block_commitment: HashMap, highest_confirmed_root: Slot, total_stake: u64, - bank: Arc, + slot: Slot, root: Slot, highest_confirmed_slot: Slot, ) -> Self { @@ -73,7 +73,7 @@ impl BlockCommitmentCache { block_commitment, highest_confirmed_root, total_stake, - bank, + slot, root, highest_confirmed_slot, } @@ -91,12 +91,8 @@ impl BlockCommitmentCache { self.total_stake } - pub fn bank(&self) -> Arc { - self.bank.clone() - } - pub fn slot(&self) -> Slot { - self.bank.slot() + self.slot } pub fn root(&self) -> Slot { @@ -153,14 +149,14 @@ impl BlockCommitmentCache { } } - pub fn new_for_tests_with_bank(bank: Arc, root: Slot) -> Self { + pub fn new_for_tests_with_slots(slot: Slot, root: Slot) -> Self { let mut block_commitment: HashMap = HashMap::new(); block_commitment.insert(0, BlockCommitment::default()); Self { block_commitment, total_stake: 42, highest_confirmed_root: root, - bank, + slot, root, highest_confirmed_slot: root, } @@ -174,7 +170,6 @@ impl BlockCommitmentCache { #[cfg(test)] mod tests { use super::*; - use solana_sdk::{genesis_config::GenesisConfig, pubkey::Pubkey}; #[test] fn test_block_commitment() { @@ -188,7 +183,6 @@ mod tests { #[test] fn test_get_confirmations() { - let bank = Arc::new(Bank::default()); // Build BlockCommitmentCache with votes at depths 0 and 1 for 2 slots let mut cache0 = BlockCommitment::default(); cache0.increase_confirmation_stake(1, 5); @@ -206,7 +200,7 @@ mod tests { block_commitment.entry(0).or_insert(cache0); block_commitment.entry(1).or_insert(cache1); block_commitment.entry(2).or_insert(cache2); - let block_commitment_cache = BlockCommitmentCache::new(block_commitment, 0, 50, bank, 0, 0); + let block_commitment_cache = BlockCommitmentCache::new(block_commitment, 0, 50, 0, 0, 0); assert_eq!(block_commitment_cache.get_confirmation_count(0), Some(2)); assert_eq!(block_commitment_cache.get_confirmation_count(1), Some(1)); @@ -216,8 +210,7 @@ mod tests { #[test] fn test_highest_confirmed_slot() { - let bank = Arc::new(Bank::new(&GenesisConfig::default())); - let bank_slot_5 = Arc::new(Bank::new_from_parent(&bank, &Pubkey::default(), 5)); + let bank_slot_5 = 5; let total_stake = 50; // Build cache with confirmation_count 2 given total_stake @@ -240,7 +233,7 @@ mod tests { block_commitment.entry(2).or_insert_with(|| cache1.clone()); // Slot 2, conf 1 block_commitment.entry(3).or_insert_with(|| cache2.clone()); // Slot 3, conf 0 let block_commitment_cache = - BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5.clone(), 0, 0); + BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5, 0, 0); assert_eq!(block_commitment_cache.calculate_highest_confirmed_slot(), 2); @@ -250,7 +243,7 @@ mod tests { block_commitment.entry(2).or_insert_with(|| cache1.clone()); // Slot 2, conf 1 block_commitment.entry(3).or_insert_with(|| cache2.clone()); // Slot 3, conf 0 let block_commitment_cache = - BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5.clone(), 0, 0); + BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5, 0, 0); assert_eq!(block_commitment_cache.calculate_highest_confirmed_slot(), 2); @@ -260,7 +253,7 @@ mod tests { block_commitment.entry(3).or_insert(cache1); // Slot 3, conf 1 block_commitment.entry(5).or_insert_with(|| cache2.clone()); // Slot 5, conf 0 let block_commitment_cache = - BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5.clone(), 0, 0); + BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5, 0, 0); assert_eq!(block_commitment_cache.calculate_highest_confirmed_slot(), 3); @@ -270,7 +263,7 @@ mod tests { block_commitment.entry(2).or_insert_with(|| cache2.clone()); // Slot 2, conf 0 block_commitment.entry(3).or_insert_with(|| cache2.clone()); // Slot 3, conf 0 let block_commitment_cache = - BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5.clone(), 0, 0); + BlockCommitmentCache::new(block_commitment, 0, total_stake, bank_slot_5, 0, 0); assert_eq!(block_commitment_cache.calculate_highest_confirmed_slot(), 1);