diff --git a/core/src/cluster_slot_state_verifier.rs b/core/src/cluster_slot_state_verifier.rs index caa91dbad2..51fe168a4f 100644 --- a/core/src/cluster_slot_state_verifier.rs +++ b/core/src/cluster_slot_state_verifier.rs @@ -18,9 +18,13 @@ pub enum SlotStateUpdate { #[derive(PartialEq, Debug)] pub enum ResultingStateChange { - MarkSlotDuplicate, + // Hash of our current frozen version of the slot + MarkSlotDuplicate(Hash), + // Hash of the cluster confirmed slot that is not equivalent + // to our frozen version of the slot RepairDuplicateConfirmedVersion(Hash), - DuplicateConfirmedSlotMatchesCluster, + // Hash of our current frozen version of the slot + DuplicateConfirmedSlotMatchesCluster(Hash), } impl SlotStateUpdate { @@ -56,9 +60,9 @@ fn on_dead_slot( // No need to check `is_slot_duplicate` and modify fork choice as dead slots // are never frozen, and thus never added to fork choice. The state change for // `MarkSlotDuplicate` will try to modify fork choice, but won't find the slot - // in the fork choice tree, so is equivalent to a + // in the fork choice tree, so is equivalent to a no-op return vec![ - ResultingStateChange::MarkSlotDuplicate, + ResultingStateChange::MarkSlotDuplicate(Hash::default()), ResultingStateChange::RepairDuplicateConfirmedVersion( *cluster_duplicate_confirmed_hash, ), @@ -92,7 +96,7 @@ fn on_frozen_slot( slot, cluster_duplicate_confirmed_hash, bank_frozen_hash ); return vec![ - ResultingStateChange::MarkSlotDuplicate, + ResultingStateChange::MarkSlotDuplicate(*bank_frozen_hash), ResultingStateChange::RepairDuplicateConfirmedVersion( *cluster_duplicate_confirmed_hash, ), @@ -101,7 +105,9 @@ fn on_frozen_slot( // If the versions match, then add the slot to the candidate // set to account for the case where it was removed earlier // by the `on_duplicate_slot()` handler - return vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster]; + return vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster( + *bank_frozen_hash, + )]; } } @@ -115,7 +121,7 @@ fn on_frozen_slot( // 2) A gossip duplicate_confirmed version that didn't match our frozen // version. // In both cases, mark the progress map for this slot as duplicate - return vec![ResultingStateChange::MarkSlotDuplicate]; + return vec![ResultingStateChange::MarkSlotDuplicate(*bank_frozen_hash)]; } vec![] @@ -202,12 +208,12 @@ fn apply_state_changes( ) { for state_change in state_changes { match state_change { - ResultingStateChange::MarkSlotDuplicate => { + ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash) => { progress.set_unconfirmed_duplicate_slot( slot, descendants.get(&slot).unwrap_or(&HashSet::default()), ); - fork_choice.mark_fork_invalid_candidate(slot); + fork_choice.mark_fork_invalid_candidate(&(slot, bank_frozen_hash)); } ResultingStateChange::RepairDuplicateConfirmedVersion( cluster_duplicate_confirmed_hash, @@ -216,13 +222,13 @@ fn apply_state_changes( // progress map from ReplayStage::confirm_forks to here. repair_correct_version(slot, &cluster_duplicate_confirmed_hash); } - ResultingStateChange::DuplicateConfirmedSlotMatchesCluster => { + ResultingStateChange::DuplicateConfirmedSlotMatchesCluster(bank_frozen_hash) => { progress.set_confirmed_duplicate_slot( slot, ancestors.get(&slot).unwrap_or(&HashSet::default()), descendants.get(&slot).unwrap_or(&HashSet::default()), ); - fork_choice.mark_fork_valid_candidate(slot); + fork_choice.mark_fork_valid_candidate(&(slot, bank_frozen_hash)); } } } @@ -388,7 +394,7 @@ mod test { is_slot_duplicate, is_dead ), - vec![ResultingStateChange::MarkSlotDuplicate] + vec![ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash)] ); } @@ -426,7 +432,9 @@ mod test { is_slot_duplicate, is_dead ), - vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster,] + vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster( + bank_frozen_hash + ),] ); // If the cluster_duplicate_confirmed_hash does not match, then we @@ -443,7 +451,7 @@ mod test { is_dead ), vec![ - ResultingStateChange::MarkSlotDuplicate, + ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash), ResultingStateChange::RepairDuplicateConfirmedVersion(mismatched_hash), ] ); @@ -483,7 +491,7 @@ mod test { is_slot_duplicate, is_dead ), - vec![ResultingStateChange::MarkSlotDuplicate,] + vec![ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash),] ); // If the cluster_duplicate_confirmed_hash matches, we just confirm @@ -497,7 +505,9 @@ mod test { is_slot_duplicate, is_dead ), - vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster,] + vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster( + bank_frozen_hash + ),] ); // If the cluster_duplicate_confirmed_hash does not match, then we @@ -514,7 +524,7 @@ mod test { is_dead ), vec![ - ResultingStateChange::MarkSlotDuplicate, + ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash), ResultingStateChange::RepairDuplicateConfirmedVersion(mismatched_hash), ] ); @@ -589,7 +599,7 @@ mod test { is_dead ), vec![ - ResultingStateChange::MarkSlotDuplicate, + ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash), ResultingStateChange::RepairDuplicateConfirmedVersion(correct_hash), ] ); @@ -605,7 +615,7 @@ mod test { is_dead ), vec![ - ResultingStateChange::MarkSlotDuplicate, + ResultingStateChange::MarkSlotDuplicate(bank_frozen_hash), ResultingStateChange::RepairDuplicateConfirmedVersion(correct_hash), ] ); @@ -620,21 +630,22 @@ mod test { ancestors, descendants, slot, - .. + bank_forks, } = setup(); // MarkSlotDuplicate should mark progress map and remove // the slot from fork choice + let slot_hash = bank_forks.read().unwrap().get(slot).unwrap().hash(); apply_state_changes( slot, &mut progress, &mut heaviest_subtree_fork_choice, &ancestors, &descendants, - vec![ResultingStateChange::MarkSlotDuplicate], + vec![ResultingStateChange::MarkSlotDuplicate(slot_hash)], ); assert!(!heaviest_subtree_fork_choice - .is_candidate_slot(slot) + .is_candidate_slot(&(slot, slot_hash)) .unwrap()); for child_slot in descendants .get(&slot) @@ -657,7 +668,9 @@ mod test { &mut heaviest_subtree_fork_choice, &ancestors, &descendants, - vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster], + vec![ResultingStateChange::DuplicateConfirmedSlotMatchesCluster( + slot_hash, + )], ); for child_slot in descendants .get(&slot) @@ -670,7 +683,7 @@ mod test { .is_none()); } assert!(heaviest_subtree_fork_choice - .is_candidate_slot(slot) + .is_candidate_slot(&(slot, slot_hash)) .unwrap()); } @@ -686,7 +699,11 @@ mod test { .. } = setup(); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 3); + let slot3_hash = bank_forks.read().unwrap().get(3).unwrap().hash(); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (3, slot3_hash) + ); let root = 0; let mut gossip_duplicate_confirmed_slots = GossipDuplicateConfirmedSlots::default(); @@ -705,13 +722,16 @@ mod test { SlotStateUpdate::DuplicateConfirmed, ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 3); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (3, slot3_hash) + ); // Mark 3 as duplicate, should not remove slot 2 from fork choice check_slot_agrees_with_cluster( 3, root, - Some(bank_forks.read().unwrap().get(3).unwrap().hash()), + Some(slot3_hash), &gossip_duplicate_confirmed_slots, &ancestors, &descendants, @@ -720,7 +740,10 @@ mod test { SlotStateUpdate::Duplicate, ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 2); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (2, slot2_hash) + ); } #[test] @@ -735,7 +758,11 @@ mod test { .. } = setup(); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 3); + let slot3_hash = bank_forks.read().unwrap().get(3).unwrap().hash(); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (3, slot3_hash) + ); let root = 0; let mut gossip_duplicate_confirmed_slots = GossipDuplicateConfirmedSlots::default(); // Mark 2 as duplicate confirmed @@ -751,10 +778,13 @@ mod test { SlotStateUpdate::Duplicate, ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 1); + let slot1_hash = bank_forks.read().unwrap().get(1).unwrap().hash(); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (1, slot1_hash) + ); // Mark slot 3 as duplicate confirmed, should mark slot 2 as duplicate confirmed as well - let slot3_hash = bank_forks.read().unwrap().get(3).unwrap().hash(); gossip_duplicate_confirmed_slots.insert(3, slot3_hash); check_slot_agrees_with_cluster( 3, @@ -768,6 +798,9 @@ mod test { SlotStateUpdate::DuplicateConfirmed, ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 3); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (3, slot3_hash) + ); } } diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 32a41e3b42..675f14bd91 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -197,8 +197,9 @@ impl Tower { ); let root = root_bank.slot(); + let (best_slot, best_hash) = heaviest_subtree_fork_choice.best_overall_slot(); let heaviest_bank = bank_forks - .get(heaviest_subtree_fork_choice.best_overall_slot()) + .get_with_checked_hash((best_slot, best_hash)) .expect( "The best overall slot must be one of `frozen_banks` which all exist in bank_forks", ) @@ -436,6 +437,10 @@ impl Tower { self.last_vote.last_voted_slot() } + pub fn last_voted_slot_hash(&self) -> Option<(Slot, Hash)> { + self.last_vote.last_voted_slot_hash() + } + pub fn stray_restored_slot(&self) -> Option { self.stray_restored_slot } @@ -1372,8 +1377,10 @@ pub mod test { } } new_bank.freeze(); - self.heaviest_subtree_fork_choice - .add_new_leaf_slot(new_bank.slot(), Some(new_bank.parent_slot())); + self.heaviest_subtree_fork_choice.add_new_leaf_slot( + (new_bank.slot(), new_bank.hash()), + Some((new_bank.parent_slot(), new_bank.parent_hash())), + ); self.bank_forks.write().unwrap().insert(new_bank); walk.forward(); } diff --git a/core/src/fork_choice.rs b/core/src/fork_choice.rs index 93f5a3d084..fde5d7da5d 100644 --- a/core/src/fork_choice.rs +++ b/core/src/fork_choice.rs @@ -4,7 +4,6 @@ use crate::{ replay_stage::HeaviestForkFailures, }; use solana_runtime::{bank::Bank, bank_forks::BankForks}; -use solana_sdk::clock::Slot; use std::{ collections::{HashMap, HashSet}, sync::{Arc, RwLock}, @@ -17,6 +16,7 @@ pub(crate) struct SelectVoteAndResetForkResult { } pub(crate) trait ForkChoice { + type ForkChoiceKey; fn compute_bank_stats( &mut self, bank: &Bank, @@ -38,7 +38,7 @@ pub(crate) trait ForkChoice { bank_forks: &RwLock, ) -> (Arc, Option>); - fn mark_fork_invalid_candidate(&mut self, invalid_slot: Slot); + fn mark_fork_invalid_candidate(&mut self, invalid_slot: &Self::ForkChoiceKey); - fn mark_fork_valid_candidate(&mut self, valid_slot: Slot); + fn mark_fork_valid_candidate(&mut self, valid_slot: &Self::ForkChoiceKey); } diff --git a/core/src/heaviest_subtree_fork_choice.rs b/core/src/heaviest_subtree_fork_choice.rs index 43d183563b..3b48109012 100644 --- a/core/src/heaviest_subtree_fork_choice.rs +++ b/core/src/heaviest_subtree_fork_choice.rs @@ -4,14 +4,17 @@ use crate::{ progress_map::ProgressMap, tree_diff::TreeDiff, }; +use solana_measure::measure::Measure; use solana_runtime::{bank::Bank, bank_forks::BankForks, epoch_stakes::EpochStakes}; use solana_sdk::{ clock::{Epoch, Slot}, epoch_schedule::EpochSchedule, + hash::Hash, pubkey::Pubkey, }; use std::{ - collections::{BTreeMap, HashMap, HashSet, VecDeque}, + borrow::Borrow, + collections::{hash_map::Entry, BTreeMap, HashMap, HashSet, VecDeque}, sync::{Arc, RwLock}, time::Instant, }; @@ -19,6 +22,9 @@ use std::{ use trees::{Tree, TreeWalk}; pub type ForkWeight = u64; +type SlotHashKey = (Slot, Hash); +type UpdateOperations = BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation>; + const MAX_ROOT_PRINT_SECONDS: u64 = 30; #[derive(PartialEq, Eq, Clone, Debug, PartialOrd, Ord)] @@ -29,12 +35,28 @@ enum UpdateLabel { Subtract, } +pub trait GetSlotHash { + fn slot_hash(&self) -> SlotHashKey; +} + +impl GetSlotHash for SlotHashKey { + fn slot_hash(&self) -> SlotHashKey { + *self + } +} + +impl GetSlotHash for Slot { + fn slot_hash(&self) -> SlotHashKey { + (*self, Hash::default()) + } +} + #[derive(PartialEq, Eq, Clone, Debug)] enum UpdateOperation { - Aggregate, Add(u64), MarkValid, Subtract(u64), + Aggregate, } impl UpdateOperation { @@ -56,23 +78,23 @@ struct ForkInfo { stake_voted_subtree: ForkWeight, // Best slot in the subtree rooted at this slot, does not // have to be a direct child in `children` - best_slot: Slot, - parent: Option, - children: Vec, + best_slot: SlotHashKey, + parent: Option, + children: Vec, // Whether the fork rooted at this slot is a valid contender // for the best fork is_candidate: bool, } pub struct HeaviestSubtreeForkChoice { - fork_infos: HashMap, - latest_votes: HashMap, - root: Slot, + fork_infos: HashMap, + latest_votes: HashMap, + root: SlotHashKey, last_root_time: Instant, } impl HeaviestSubtreeForkChoice { - pub(crate) fn new(root: Slot) -> Self { + pub(crate) fn new(root: SlotHashKey) -> Self { let mut heaviest_subtree_fork_choice = Self { root, // Doesn't implement default because `root` must @@ -87,17 +109,23 @@ impl HeaviestSubtreeForkChoice { // Given a root and a list of `frozen_banks` sorted smallest to greatest by slot, // return a new HeaviestSubtreeForkChoice - pub(crate) fn new_from_frozen_banks(root: Slot, frozen_banks: &[Arc]) -> Self { + pub(crate) fn new_from_frozen_banks(root: SlotHashKey, frozen_banks: &[Arc]) -> Self { let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new(root); - let mut prev_slot = root; + let mut prev_slot = root.0; for bank in frozen_banks.iter() { assert!(bank.is_frozen()); - if bank.slot() > root { + if bank.slot() > root.0 { // Make sure the list is sorted assert!(bank.slot() > prev_slot); prev_slot = bank.slot(); - heaviest_subtree_fork_choice - .add_new_leaf_slot(bank.slot(), Some(bank.parent_slot())); + let bank_hash = bank.hash(); + assert_ne!(bank_hash, Hash::default()); + let parent_bank_hash = bank.parent_hash(); + assert_ne!(parent_bank_hash, Hash::default()); + heaviest_subtree_fork_choice.add_new_leaf_slot( + (bank.slot(), bank_hash), + Some((bank.parent_slot(), parent_bank_hash)), + ); } } @@ -109,58 +137,61 @@ impl HeaviestSubtreeForkChoice { let mut frozen_banks: Vec<_> = bank_forks.frozen_banks().values().cloned().collect(); frozen_banks.sort_by_key(|bank| bank.slot()); - let root = bank_forks.root(); - Self::new_from_frozen_banks(root, &frozen_banks) + let root_bank = bank_forks.root_bank(); + Self::new_from_frozen_banks((root_bank.slot(), root_bank.hash()), &frozen_banks) } #[cfg(test)] - pub(crate) fn new_from_tree(forks: Tree) -> Self { - let root = forks.root().data; + pub(crate) fn new_from_tree(forks: Tree) -> Self { + let root = forks.root().data.slot_hash(); let mut walk = TreeWalk::from(forks); let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new(root); while let Some(visit) = walk.get() { - let slot = visit.node().data; - if heaviest_subtree_fork_choice.fork_infos.contains_key(&slot) { + let slot_hash = visit.node().data.slot_hash(); + if heaviest_subtree_fork_choice + .fork_infos + .contains_key(&slot_hash) + { walk.forward(); continue; } - let parent = walk.get_parent().map(|n| n.data); - heaviest_subtree_fork_choice.add_new_leaf_slot(slot, parent); + let parent_slot_hash = walk.get_parent().map(|n| n.data.slot_hash()); + heaviest_subtree_fork_choice.add_new_leaf_slot(slot_hash, parent_slot_hash); walk.forward(); } heaviest_subtree_fork_choice } - pub fn best_slot(&self, slot: Slot) -> Option { + pub fn best_slot(&self, key: &SlotHashKey) -> Option { self.fork_infos - .get(&slot) + .get(key) .map(|fork_info| fork_info.best_slot) } - pub fn best_overall_slot(&self) -> Slot { - self.best_slot(self.root).unwrap() + pub fn best_overall_slot(&self) -> SlotHashKey { + self.best_slot(&self.root).unwrap() } - pub fn stake_voted_subtree(&self, slot: Slot) -> Option { + pub fn stake_voted_subtree(&self, key: &SlotHashKey) -> Option { self.fork_infos - .get(&slot) + .get(key) .map(|fork_info| fork_info.stake_voted_subtree) } - pub fn is_candidate_slot(&self, slot: Slot) -> Option { + pub fn is_candidate_slot(&self, key: &SlotHashKey) -> Option { self.fork_infos - .get(&slot) + .get(key) .map(|fork_info| fork_info.is_candidate) } - pub fn root(&self) -> Slot { + pub fn root(&self) -> SlotHashKey { self.root } - pub fn max_by_weight(&self, slot1: Slot, slot2: Slot) -> std::cmp::Ordering { - let weight1 = self.stake_voted_subtree(slot1).unwrap(); - let weight2 = self.stake_voted_subtree(slot2).unwrap(); + pub fn max_by_weight(&self, slot1: SlotHashKey, slot2: SlotHashKey) -> std::cmp::Ordering { + let weight1 = self.stake_voted_subtree(&slot1).unwrap(); + let weight2 = self.stake_voted_subtree(&slot2).unwrap(); if weight1 == weight2 { slot1.cmp(&slot2).reverse() } else { @@ -169,41 +200,42 @@ impl HeaviestSubtreeForkChoice { } // Add new votes, returns the best slot - pub fn add_votes( - &mut self, + pub fn add_votes<'a, 'b>( + &'a mut self, // newly updated votes on a fork - pubkey_votes: &[(Pubkey, Slot)], + pubkey_votes: impl Iterator + 'b>, epoch_stakes: &HashMap, epoch_schedule: &EpochSchedule, - ) -> Slot { + ) -> SlotHashKey { // Generate the set of updates - let update_operations = + let update_operations_batch = self.generate_update_operations(pubkey_votes, epoch_stakes, epoch_schedule); // Finalize all updates - self.process_update_operations(update_operations); + self.process_update_operations(update_operations_batch); self.best_overall_slot() } - pub fn set_root(&mut self, new_root: Slot) { + pub fn set_root(&mut self, new_root: SlotHashKey) { // Remove everything reachable from `self.root` but not `new_root`, // as those are now unrooted. let remove_set = self.subtree_diff(self.root, new_root); - for slot in remove_set { + for node_key in remove_set { self.fork_infos - .remove(&slot) + .remove(&node_key) .expect("Slots reachable from old root must exist in tree"); } - self.fork_infos - .get_mut(&new_root) - .expect("new root must exist in fork_infos map") + let root_fork_info = self.fork_infos.get_mut(&new_root); + + root_fork_info + .unwrap_or_else(|| panic!("New root: {:?}, didn't exist in fork choice", new_root)) .parent = None; self.root = new_root; self.last_root_time = Instant::now(); } - pub fn add_root_parent(&mut self, root_parent: Slot) { - assert!(root_parent < self.root); + pub fn add_root_parent(&mut self, root_parent: SlotHashKey) { + assert!(root_parent.0 < self.root.0); assert!(self.fork_infos.get(&root_parent).is_none()); let root_info = self .fork_infos @@ -223,12 +255,18 @@ impl HeaviestSubtreeForkChoice { self.root = root_parent; } - pub fn add_new_leaf_slot(&mut self, slot: Slot, parent: Option) { + pub fn add_new_leaf_slot(&mut self, slot: SlotHashKey, parent: Option) { if self.last_root_time.elapsed().as_secs() > MAX_ROOT_PRINT_SECONDS { self.print_state(); self.last_root_time = Instant::now(); } + if self.fork_infos.contains_key(&slot) { + // Can potentially happen if we repair the same version of the duplicate slot, after + // dumping the original version + return; + } + self.fork_infos .entry(slot) .and_modify(|slot_info| slot_info.parent = parent) @@ -257,33 +295,33 @@ impl HeaviestSubtreeForkChoice { // Propagate leaf up the tree to any ancestors who considered the previous leaf // the `best_slot` - self.propagate_new_leaf(slot, parent) + self.propagate_new_leaf(&slot, &parent) } // Returns if the given `maybe_best_child` is the heaviest among the children // it's parent - fn is_best_child(&self, maybe_best_child: Slot) -> bool { + fn is_best_child(&self, maybe_best_child: &SlotHashKey) -> bool { let maybe_best_child_weight = self.stake_voted_subtree(maybe_best_child).unwrap(); let parent = self.parent(maybe_best_child); // If there's no parent, this must be the root if parent.is_none() { return true; } - for child in self.children(parent.unwrap()).unwrap() { + for child in self.children(&parent.unwrap()).unwrap() { let child_weight = self - .stake_voted_subtree(*child) + .stake_voted_subtree(child) .expect("child must exist in `self.fork_infos`"); // Don't count children currently marked as invalid if !self - .is_candidate_slot(*child) + .is_candidate_slot(child) .expect("child must exist in tree") { continue; } if child_weight > maybe_best_child_weight - || (maybe_best_child_weight == child_weight && *child < maybe_best_child) + || (maybe_best_child_weight == child_weight && *child < *maybe_best_child) { return false; } @@ -292,76 +330,71 @@ impl HeaviestSubtreeForkChoice { true } - pub fn all_slots_stake_voted_subtree(&self) -> Vec<(Slot, u64)> { + pub fn all_slots_stake_voted_subtree(&self) -> impl Iterator { self.fork_infos .iter() - .map(|(slot, fork_info)| (*slot, fork_info.stake_voted_subtree)) - .collect() + .map(|(slot_hash, fork_info)| (slot_hash, fork_info.stake_voted_subtree)) } - pub fn ancestors(&self, start_slot: Slot) -> Vec { - AncestorIterator::new(start_slot, &self.fork_infos).collect() + #[cfg(test)] + pub fn ancestors(&self, start_slot_hash_key: SlotHashKey) -> Vec { + AncestorIterator::new(start_slot_hash_key, &self.fork_infos).collect() } pub fn merge( &mut self, other: HeaviestSubtreeForkChoice, - merge_leaf: Slot, + merge_leaf: &SlotHashKey, epoch_stakes: &HashMap, epoch_schedule: &EpochSchedule, ) { - assert!(self.fork_infos.contains_key(&merge_leaf)); + assert!(self.fork_infos.contains_key(merge_leaf)); // Add all the nodes from `other` into our tree let mut other_slots_nodes: Vec<_> = other .fork_infos .iter() - .map(|(slot, fork_info)| (slot, fork_info.parent.unwrap_or(merge_leaf))) - .collect(); - - other_slots_nodes.sort_by_key(|(slot, _)| *slot); - for (slot, parent) in other_slots_nodes { - self.add_new_leaf_slot(*slot, Some(parent)); - } - - // Add all the latest votes from `other` that are newer than the ones - // in the current tree - let new_votes: Vec<_> = other - .latest_votes - .into_iter() - .filter(|(pk, other_latest_slot)| { - self.latest_votes - .get(&pk) - .map(|latest_slot| other_latest_slot > latest_slot) - .unwrap_or(false) + .map(|(slot_hash_key, fork_info)| { + (slot_hash_key, fork_info.parent.unwrap_or(*merge_leaf)) }) .collect(); - self.add_votes(&new_votes, epoch_stakes, epoch_schedule); + other_slots_nodes.sort_by_key(|(slot_hash_key, _)| *slot_hash_key); + for (slot_hash_key, parent) in other_slots_nodes { + self.add_new_leaf_slot(*slot_hash_key, Some(parent)); + } + + // Add all votes, the outdated ones should be filtered out by + // self.add_votes() + self.add_votes(other.latest_votes.into_iter(), epoch_stakes, epoch_schedule); } - pub fn stake_voted_at(&self, slot: Slot) -> Option { + pub fn stake_voted_at(&self, slot: &SlotHashKey) -> Option { self.fork_infos - .get(&slot) + .get(slot) .map(|fork_info| fork_info.stake_voted_at) } - fn propagate_new_leaf(&mut self, slot: Slot, parent: Slot) { - let parent_best_slot = self - .best_slot(parent) + fn propagate_new_leaf( + &mut self, + slot_hash_key: &SlotHashKey, + parent_slot_hash_key: &SlotHashKey, + ) { + let parent_best_slot_hash_key = self + .best_slot(parent_slot_hash_key) .expect("parent must exist in self.fork_infos after its child leaf was created"); // If this new leaf is the direct parent's best child, then propagate // it up the tree - if self.is_best_child(slot) { - let mut ancestor = Some(parent); + if self.is_best_child(slot_hash_key) { + let mut ancestor = Some(*parent_slot_hash_key); loop { if ancestor.is_none() { break; } let ancestor_fork_info = self.fork_infos.get_mut(&ancestor.unwrap()).unwrap(); - if ancestor_fork_info.best_slot == parent_best_slot { - ancestor_fork_info.best_slot = slot; + if ancestor_fork_info.best_slot == parent_best_slot_hash_key { + ancestor_fork_info.best_slot = *slot_hash_key; } else { break; } @@ -372,56 +405,59 @@ impl HeaviestSubtreeForkChoice { fn insert_mark_valid_aggregate_operations( &self, - update_operations: &mut BTreeMap<(Slot, UpdateLabel), UpdateOperation>, - slot: Slot, + update_operations: &mut BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation>, + slot_hash_key: SlotHashKey, ) { - self.do_insert_aggregate_operations(update_operations, true, slot); + self.do_insert_aggregate_operations(update_operations, true, slot_hash_key); } fn insert_aggregate_operations( &self, - update_operations: &mut BTreeMap<(Slot, UpdateLabel), UpdateOperation>, - slot: Slot, + update_operations: &mut BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation>, + slot_hash_key: SlotHashKey, ) { - self.do_insert_aggregate_operations(update_operations, false, slot); + self.do_insert_aggregate_operations(update_operations, false, slot_hash_key); } #[allow(clippy::map_entry)] fn do_insert_aggregate_operations( &self, - update_operations: &mut BTreeMap<(Slot, UpdateLabel), UpdateOperation>, + update_operations: &mut BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation>, should_mark_valid: bool, - slot: Slot, + slot_hash_key: SlotHashKey, ) { - for parent in self.ancestor_iterator(slot) { - let aggregate_label = (parent, UpdateLabel::Aggregate); + for parent_slot_hash_key in self.ancestor_iterator(slot_hash_key) { + let aggregate_label = (parent_slot_hash_key, UpdateLabel::Aggregate); if update_operations.contains_key(&aggregate_label) { break; } else { if should_mark_valid { - update_operations - .insert((parent, UpdateLabel::MarkValid), UpdateOperation::MarkValid); + update_operations.insert( + (parent_slot_hash_key, UpdateLabel::MarkValid), + UpdateOperation::MarkValid, + ); } update_operations.insert(aggregate_label, UpdateOperation::Aggregate); } } } - fn ancestor_iterator(&self, start_slot: Slot) -> AncestorIterator { - AncestorIterator::new(start_slot, &self.fork_infos) + fn ancestor_iterator(&self, start_slot_hash_key: SlotHashKey) -> AncestorIterator { + AncestorIterator::new(start_slot_hash_key, &self.fork_infos) } - fn aggregate_slot(&mut self, slot: Slot) { + fn aggregate_slot(&mut self, slot_hash_key: SlotHashKey) { let mut stake_voted_subtree; - let mut best_slot = slot; - if let Some(fork_info) = self.fork_infos.get(&slot) { + let mut best_slot_hash_key = slot_hash_key; + if let Some(fork_info) = self.fork_infos.get(&slot_hash_key) { stake_voted_subtree = fork_info.stake_voted_at; let mut best_child_stake_voted_subtree = 0; - let mut best_child_slot = slot; - for &child in &fork_info.children { + let mut best_child_slot = slot_hash_key; + for child in &fork_info.children { let child_stake_voted_subtree = self.stake_voted_subtree(child).unwrap(); + // Child forks that are not candidates still contribute to the weight - // of the subtree rooted at `slot`. For instance: + // of the subtree rooted at `slot_hash_key`. For instance: /* Build fork structure: slot 0 @@ -446,143 +482,199 @@ impl HeaviestSubtreeForkChoice { if self .is_candidate_slot(child) .expect("Child must exist in fork_info map") - && (best_child_slot == slot || + && (best_child_slot == slot_hash_key || child_stake_voted_subtree > best_child_stake_voted_subtree || // tiebreaker by slot height, prioritize earlier slot - (child_stake_voted_subtree == best_child_stake_voted_subtree && child < best_child_slot)) + (child_stake_voted_subtree == best_child_stake_voted_subtree && child < &best_child_slot)) { - { - best_child_stake_voted_subtree = child_stake_voted_subtree; - best_child_slot = child; - best_slot = self - .best_slot(child) - .expect("`child` must exist in `self.fork_infos`"); - } + best_child_stake_voted_subtree = child_stake_voted_subtree; + best_child_slot = *child; + best_slot_hash_key = self + .best_slot(child) + .expect("`child` must exist in `self.fork_infos`"); } } } else { return; } - let fork_info = self.fork_infos.get_mut(&slot).unwrap(); + let fork_info = self.fork_infos.get_mut(&slot_hash_key).unwrap(); fork_info.stake_voted_subtree = stake_voted_subtree; - fork_info.best_slot = best_slot; + fork_info.best_slot = best_slot_hash_key; } - fn mark_slot_valid(&mut self, valid_slot: Slot) { - if let Some(fork_info) = self.fork_infos.get_mut(&valid_slot) { + fn mark_slot_valid(&mut self, valid_slot_hash_key: (Slot, Hash)) { + if let Some(fork_info) = self.fork_infos.get_mut(&valid_slot_hash_key) { if !fork_info.is_candidate { info!( - "marked previously invalid fork starting at slot: {} as valid", - valid_slot + "marked previously invalid fork starting at slot: {:?} as valid", + valid_slot_hash_key ); } fork_info.is_candidate = true; } } - fn generate_update_operations( - &mut self, - pubkey_votes: &[(Pubkey, Slot)], + fn generate_update_operations<'a, 'b>( + &'a mut self, + pubkey_votes: impl Iterator + 'b>, epoch_stakes: &HashMap, epoch_schedule: &EpochSchedule, - ) -> BTreeMap<(Slot, UpdateLabel), UpdateOperation> { - let mut update_operations: BTreeMap<(Slot, UpdateLabel), UpdateOperation> = BTreeMap::new(); - + ) -> UpdateOperations { + let mut update_operations: BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation> = + BTreeMap::new(); + let mut observed_pubkeys: HashMap = HashMap::new(); // Sort the `pubkey_votes` in a BTreeMap by the slot voted - for &(pubkey, new_vote_slot) in pubkey_votes.iter() { - let pubkey_latest_vote = self.latest_votes.get(&pubkey).unwrap_or(&0); - // Filter out any votes or slots <= any slot this pubkey has - // already voted for, we only care about the latest votes - if new_vote_slot <= *pubkey_latest_vote { + for pubkey_vote in pubkey_votes { + let (pubkey, new_vote_slot_hash) = pubkey_vote.borrow(); + let (new_vote_slot, new_vote_hash) = *new_vote_slot_hash; + if new_vote_slot < self.root.0 { continue; } - // Remove this pubkey stake from previous fork - if let Some(old_latest_vote_slot) = self.latest_votes.insert(pubkey, new_vote_slot) { - let epoch = epoch_schedule.get_epoch(old_latest_vote_slot); - let stake_update = epoch_stakes - .get(&epoch) - .map(|epoch_stakes| epoch_stakes.vote_account_stake(&pubkey)) - .unwrap_or(0); - if stake_update > 0 { - update_operations - .entry((old_latest_vote_slot, UpdateLabel::Subtract)) - .and_modify(|update| update.update_stake(stake_update)) - .or_insert(UpdateOperation::Subtract(stake_update)); - self.insert_aggregate_operations(&mut update_operations, old_latest_vote_slot); + // A pubkey cannot appear in pubkey votes more than once. + match observed_pubkeys.entry(*pubkey) { + Entry::Occupied(_occupied_entry) => { + panic!("Should not get multiple votes for same pubkey in the same batch"); + } + Entry::Vacant(vacant_entry) => { + vacant_entry.insert(new_vote_slot); + false + } + }; + + let mut pubkey_latest_vote = self.latest_votes.get_mut(pubkey); + + // Filter out any votes or slots < any slot this pubkey has + // already voted for, we only care about the latest votes. + // + // If the new vote is for the same slot, but a different, smaller hash, + // then allow processing to continue as this is a duplicate version + // of the same slot. + match pubkey_latest_vote.as_mut() { + Some((pubkey_latest_vote_slot, pubkey_latest_vote_hash)) + if (new_vote_slot < *pubkey_latest_vote_slot) + || (new_vote_slot == *pubkey_latest_vote_slot + && &new_vote_hash >= pubkey_latest_vote_hash) => + { + continue; + } + + _ => { + // We either: + // 1) don't have a vote yet for this pubkey, + // 2) or the new vote slot is bigger than the old vote slot + // 3) or the new vote slot == old_vote slot, but for a smaller bank hash. + // In all above cases, we need to remove this pubkey stake from the previous fork + // of the previous vote + + if let Some((old_latest_vote_slot, old_latest_vote_hash)) = + self.latest_votes.insert(*pubkey, *new_vote_slot_hash) + { + assert!(if new_vote_slot == old_latest_vote_slot { + warn!( + "Got a duplicate vote for + validator: {}, + slot_hash: {:?}", + pubkey, new_vote_slot_hash + ); + // If the slots are equal, then the new + // vote must be for a smaller hash + new_vote_hash < old_latest_vote_hash + } else { + new_vote_slot > old_latest_vote_slot + }); + + let epoch = epoch_schedule.get_epoch(old_latest_vote_slot); + let stake_update = epoch_stakes + .get(&epoch) + .map(|epoch_stakes| epoch_stakes.vote_account_stake(pubkey)) + .unwrap_or(0); + + if stake_update > 0 { + update_operations + .entry(( + (old_latest_vote_slot, old_latest_vote_hash), + UpdateLabel::Subtract, + )) + .and_modify(|update| update.update_stake(stake_update)) + .or_insert(UpdateOperation::Subtract(stake_update)); + self.insert_aggregate_operations( + &mut update_operations, + (old_latest_vote_slot, old_latest_vote_hash), + ); + } + } } } // Add this pubkey stake to new fork - let epoch = epoch_schedule.get_epoch(new_vote_slot); + let epoch = epoch_schedule.get_epoch(new_vote_slot_hash.0); let stake_update = epoch_stakes .get(&epoch) .map(|epoch_stakes| epoch_stakes.vote_account_stake(&pubkey)) .unwrap_or(0); + update_operations - .entry((new_vote_slot, UpdateLabel::Add)) + .entry((*new_vote_slot_hash, UpdateLabel::Add)) .and_modify(|update| update.update_stake(stake_update)) .or_insert(UpdateOperation::Add(stake_update)); - self.insert_aggregate_operations(&mut update_operations, new_vote_slot); + self.insert_aggregate_operations(&mut update_operations, *new_vote_slot_hash); } update_operations } - fn process_update_operations( - &mut self, - update_operations: BTreeMap<(Slot, UpdateLabel), UpdateOperation>, - ) { + fn process_update_operations(&mut self, update_operations: UpdateOperations) { // Iterate through the update operations from greatest to smallest slot - for ((slot, _), operation) in update_operations.into_iter().rev() { + for ((slot_hash_key, _), operation) in update_operations.into_iter().rev() { match operation { - UpdateOperation::MarkValid => self.mark_slot_valid(slot), - UpdateOperation::Aggregate => self.aggregate_slot(slot), - UpdateOperation::Add(stake) => self.add_slot_stake(slot, stake), - UpdateOperation::Subtract(stake) => self.subtract_slot_stake(slot, stake), + UpdateOperation::MarkValid => self.mark_slot_valid(slot_hash_key), + UpdateOperation::Aggregate => self.aggregate_slot(slot_hash_key), + UpdateOperation::Add(stake) => self.add_slot_stake(&slot_hash_key, stake), + UpdateOperation::Subtract(stake) => self.subtract_slot_stake(&slot_hash_key, stake), } } } - fn add_slot_stake(&mut self, slot: Slot, stake: u64) { - if let Some(fork_info) = self.fork_infos.get_mut(&slot) { + fn add_slot_stake(&mut self, slot_hash_key: &SlotHashKey, stake: u64) { + if let Some(fork_info) = self.fork_infos.get_mut(slot_hash_key) { fork_info.stake_voted_at += stake; fork_info.stake_voted_subtree += stake; } } - fn subtract_slot_stake(&mut self, slot: Slot, stake: u64) { - if let Some(fork_info) = self.fork_infos.get_mut(&slot) { + fn subtract_slot_stake(&mut self, slot_hash_key: &SlotHashKey, stake: u64) { + if let Some(fork_info) = self.fork_infos.get_mut(slot_hash_key) { fork_info.stake_voted_at -= stake; fork_info.stake_voted_subtree -= stake; } } - fn parent(&self, slot: Slot) -> Option { + fn parent(&self, slot_hash_key: &SlotHashKey) -> Option { self.fork_infos - .get(&slot) + .get(slot_hash_key) .map(|fork_info| fork_info.parent) .unwrap_or(None) } fn print_state(&self) { - let best_slot = self.best_overall_slot(); - let mut best_path: VecDeque<_> = self.ancestor_iterator(best_slot).collect(); - best_path.push_front(best_slot); + let best_slot_hash_key = self.best_overall_slot(); + let mut best_path: VecDeque<_> = self.ancestor_iterator(best_slot_hash_key).collect(); + best_path.push_front(best_slot_hash_key); info!( "Latest known votes by vote pubkey: {:#?}, best path: {:?}", self.latest_votes, - best_path.iter().rev().collect::>() + best_path.iter().rev().collect::>() ); } - fn heaviest_slot_on_same_voted_fork(&self, tower: &Tower) -> Option { + fn heaviest_slot_on_same_voted_fork(&self, tower: &Tower) -> Option { tower - .last_voted_slot() - .map(|last_voted_slot| { - let heaviest_slot_on_same_voted_fork = self.best_slot(last_voted_slot); - if heaviest_slot_on_same_voted_fork.is_none() { + .last_voted_slot_hash() + .and_then(|last_voted_slot_hash| { + let heaviest_slot_hash_on_same_voted_fork = self.best_slot(&last_voted_slot_hash); + if heaviest_slot_hash_on_same_voted_fork.is_none() { if !tower.is_stray_last_vote() { // Unless last vote is stray and stale, self.bast_slot(last_voted_slot) must return // Some(_), justifying to panic! here. @@ -593,9 +685,9 @@ impl HeaviestSubtreeForkChoice { // validator has been running, so we must be able to fetch best_slots for all of // them. panic!( - "a bank at last_voted_slot({}) is a frozen bank so must have been \ + "a bank at last_voted_slot({:?}) is a frozen bank so must have been \ added to heaviest_subtree_fork_choice at time of freezing", - last_voted_slot, + last_voted_slot_hash, ) } else { // fork_infos doesn't have corresponding data for the stale stray last vote, @@ -604,61 +696,88 @@ impl HeaviestSubtreeForkChoice { return None; } } - let heaviest_slot_on_same_voted_fork = heaviest_slot_on_same_voted_fork.unwrap(); + let heaviest_slot_hash_on_same_voted_fork = + heaviest_slot_hash_on_same_voted_fork.unwrap(); - if heaviest_slot_on_same_voted_fork == last_voted_slot { + if heaviest_slot_hash_on_same_voted_fork == last_voted_slot_hash { None } else { - Some(heaviest_slot_on_same_voted_fork) + Some(heaviest_slot_hash_on_same_voted_fork) } }) - .unwrap_or(None) } #[cfg(test)] - fn set_stake_voted_at(&mut self, slot: Slot, stake_voted_at: u64) { - self.fork_infos.get_mut(&slot).unwrap().stake_voted_at = stake_voted_at; + fn set_stake_voted_at(&mut self, slot_hash_key: SlotHashKey, stake_voted_at: u64) { + self.fork_infos + .get_mut(&slot_hash_key) + .unwrap() + .stake_voted_at = stake_voted_at; } #[cfg(test)] - fn is_leaf(&self, slot: Slot) -> bool { - self.fork_infos.get(&slot).unwrap().children.is_empty() + fn is_leaf(&self, slot_hash_key: SlotHashKey) -> bool { + self.fork_infos + .get(&slot_hash_key) + .unwrap() + .children + .is_empty() } } impl TreeDiff for HeaviestSubtreeForkChoice { - fn contains_slot(&self, slot: Slot) -> bool { - self.fork_infos.contains_key(&slot) + type TreeKey = SlotHashKey; + fn contains_slot(&self, slot_hash_key: &SlotHashKey) -> bool { + self.fork_infos.contains_key(slot_hash_key) } - fn children(&self, slot: Slot) -> Option<&[Slot]> { + fn children(&self, slot_hash_key: &SlotHashKey) -> Option<&[SlotHashKey]> { self.fork_infos - .get(&slot) + .get(&slot_hash_key) .map(|fork_info| &fork_info.children[..]) } } impl ForkChoice for HeaviestSubtreeForkChoice { + type ForkChoiceKey = SlotHashKey; fn compute_bank_stats( &mut self, bank: &Bank, _tower: &Tower, - _progress: &mut ProgressMap, + progress: &mut ProgressMap, computed_bank_state: &ComputedBankState, ) { let ComputedBankState { pubkey_votes, .. } = computed_bank_state; - + let mut start = Measure::start("compute_bank_stats_time"); // Update `heaviest_subtree_fork_choice` to find the best fork to build on - let best_overall_slot = self.add_votes( - &pubkey_votes, + let root = self.root.0; + let (best_overall_slot, best_overall_hash) = self.add_votes( + pubkey_votes.iter().filter_map(|(pubkey, slot)| { + if *slot >= root { + Some(( + *pubkey, + ( + *slot, + progress + .get_hash(*slot) + .expect("Votes for ancestors must exist in progress map"), + ), + )) + } else { + None + } + }), bank.epoch_stakes_map(), bank.epoch_schedule(), ); + start.stop(); datapoint_info!( - "best_slot", - ("slot", bank.slot(), i64), - ("best_slot", best_overall_slot, i64), + "compute_bank_stats-best_slot", + ("computed_slot", bank.slot(), i64), + ("overall_best_slot", best_overall_slot, i64), + ("overall_best_hash", best_overall_hash.to_string(), String), + ("elapsed", start.as_us(), i64), ); } @@ -675,78 +794,88 @@ impl ForkChoice for HeaviestSubtreeForkChoice { bank_forks: &RwLock, ) -> (Arc, Option>) { let r_bank_forks = bank_forks.read().unwrap(); - ( - r_bank_forks.get(self.best_overall_slot()).unwrap().clone(), + // BankForks should only contain one valid version of this slot + r_bank_forks + .get_with_checked_hash(self.best_overall_slot()) + .unwrap() + .clone(), self.heaviest_slot_on_same_voted_fork(tower) - .map(|heaviest_slot_on_same_voted_fork| { + .map(|slot_hash| { + // BankForks should only contain one valid version of this slot r_bank_forks - .get(heaviest_slot_on_same_voted_fork) + .get_with_checked_hash(slot_hash) .unwrap() .clone() }), ) } - fn mark_fork_invalid_candidate(&mut self, invalid_slot: Slot) { + fn mark_fork_invalid_candidate(&mut self, invalid_slot_hash_key: &SlotHashKey) { info!( - "marking fork starting at slot: {} invalid candidate", - invalid_slot + "marking fork starting at slot: {:?} invalid candidate", + invalid_slot_hash_key ); - let fork_info = self.fork_infos.get_mut(&invalid_slot); + let fork_info = self.fork_infos.get_mut(invalid_slot_hash_key); if let Some(fork_info) = fork_info { if fork_info.is_candidate { fork_info.is_candidate = false; // Aggregate to find the new best slots excluding this fork - let mut aggregate_operations = BTreeMap::new(); - self.insert_aggregate_operations(&mut aggregate_operations, invalid_slot); - self.process_update_operations(aggregate_operations); + let mut update_operations = UpdateOperations::default(); + self.insert_aggregate_operations(&mut update_operations, *invalid_slot_hash_key); + self.process_update_operations(update_operations); } } } - fn mark_fork_valid_candidate(&mut self, valid_slot: Slot) { - let mut aggregate_operations = BTreeMap::new(); - let fork_info = self.fork_infos.get_mut(&valid_slot); + fn mark_fork_valid_candidate(&mut self, valid_slot_hash_key: &SlotHashKey) { + let mut update_operations = UpdateOperations::default(); + let fork_info = self.fork_infos.get_mut(valid_slot_hash_key); if let Some(fork_info) = fork_info { // If a bunch of slots on the same fork are confirmed at once, then only the latest // slot will incur this aggregation operation fork_info.is_candidate = true; - self.insert_mark_valid_aggregate_operations(&mut aggregate_operations, valid_slot); + self.insert_mark_valid_aggregate_operations( + &mut update_operations, + *valid_slot_hash_key, + ); } // Aggregate to find the new best slots including this fork - self.process_update_operations(aggregate_operations); + self.process_update_operations(update_operations); } } struct AncestorIterator<'a> { - current_slot: Slot, - fork_infos: &'a HashMap, + current_slot_hash_key: SlotHashKey, + fork_infos: &'a HashMap, } impl<'a> AncestorIterator<'a> { - fn new(start_slot: Slot, fork_infos: &'a HashMap) -> Self { + fn new( + start_slot_hash_key: SlotHashKey, + fork_infos: &'a HashMap, + ) -> Self { Self { - current_slot: start_slot, + current_slot_hash_key: start_slot_hash_key, fork_infos, } } } impl<'a> Iterator for AncestorIterator<'a> { - type Item = Slot; + type Item = SlotHashKey; fn next(&mut self) -> Option { - let parent = self + let parent_slot_hash_key = self .fork_infos - .get(&self.current_slot) + .get(&self.current_slot_hash_key) .map(|fork_info| fork_info.parent) .unwrap_or(None); - parent - .map(|parent| { - self.current_slot = parent; - Some(self.current_slot) + parent_slot_hash_key + .map(|parent_slot_hash_key| { + self.current_slot_hash_key = parent_slot_hash_key; + Some(self.current_slot_hash_key) }) .unwrap_or(None) } @@ -777,17 +906,17 @@ mod test { let stake = 100; let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(1, stake); heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 4)], + [(vote_pubkeys[0], (4, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); assert_eq!( - heaviest_subtree_fork_choice.max_by_weight(4, 5), + heaviest_subtree_fork_choice.max_by_weight((4, Hash::default()), (5, Hash::default())), std::cmp::Ordering::Greater ); assert_eq!( - heaviest_subtree_fork_choice.max_by_weight(4, 0), + heaviest_subtree_fork_choice.max_by_weight((4, Hash::default()), (0, Hash::default())), std::cmp::Ordering::Less ); } @@ -807,41 +936,98 @@ mod test { let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(1, stake); let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new_from_tree(forks); heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 5)], + [(vote_pubkeys[0], (5, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - heaviest_subtree_fork_choice.add_root_parent(2); - assert_eq!(heaviest_subtree_fork_choice.parent(3).unwrap(), 2); + heaviest_subtree_fork_choice.add_root_parent((2, Hash::default())); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_subtree(3).unwrap(), + heaviest_subtree_fork_choice + .parent(&(3, Hash::default())) + .unwrap(), + (2, Hash::default()) + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&(3, Hash::default())) + .unwrap(), stake ); - assert_eq!(heaviest_subtree_fork_choice.stake_voted_at(2).unwrap(), 0); assert_eq!( - heaviest_subtree_fork_choice.children(2).unwrap().to_vec(), - vec![3] + heaviest_subtree_fork_choice + .stake_voted_at(&(2, Hash::default())) + .unwrap(), + 0 ); - assert_eq!(heaviest_subtree_fork_choice.best_slot(2).unwrap(), 5); - assert!(heaviest_subtree_fork_choice.parent(2).is_none()); + assert_eq!( + heaviest_subtree_fork_choice + .children(&(2, Hash::default())) + .unwrap() + .to_vec(), + vec![(3, Hash::default())] + ); + assert_eq!( + heaviest_subtree_fork_choice + .best_slot(&(2, Hash::default())) + .unwrap() + .0, + 5 + ); + assert!(heaviest_subtree_fork_choice + .parent(&(2, Hash::default())) + .is_none()); } #[test] fn test_ancestor_iterator() { let mut heaviest_subtree_fork_choice = setup_forks(); - let parents: Vec<_> = heaviest_subtree_fork_choice.ancestor_iterator(6).collect(); - assert_eq!(parents, vec![5, 3, 1, 0]); - let parents: Vec<_> = heaviest_subtree_fork_choice.ancestor_iterator(4).collect(); - assert_eq!(parents, vec![2, 1, 0]); - let parents: Vec<_> = heaviest_subtree_fork_choice.ancestor_iterator(1).collect(); - assert_eq!(parents, vec![0]); - let parents: Vec<_> = heaviest_subtree_fork_choice.ancestor_iterator(0).collect(); + let parents: Vec<_> = heaviest_subtree_fork_choice + .ancestor_iterator((6, Hash::default())) + .collect(); + assert_eq!( + parents, + vec![5, 3, 1, 0] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() + ); + let parents: Vec<_> = heaviest_subtree_fork_choice + .ancestor_iterator((4, Hash::default())) + .collect(); + assert_eq!( + parents, + vec![2, 1, 0] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() + ); + let parents: Vec<_> = heaviest_subtree_fork_choice + .ancestor_iterator((1, Hash::default())) + .collect(); + assert_eq!( + parents, + vec![0] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() + ); + let parents: Vec<_> = heaviest_subtree_fork_choice + .ancestor_iterator((0, Hash::default())) + .collect(); assert!(parents.is_empty()); // Set a root, everything but slots 2, 4 should be removed - heaviest_subtree_fork_choice.set_root(2); - let parents: Vec<_> = heaviest_subtree_fork_choice.ancestor_iterator(4).collect(); - assert_eq!(parents, vec![2]); + heaviest_subtree_fork_choice.set_root((2, Hash::default())); + let parents: Vec<_> = heaviest_subtree_fork_choice + .ancestor_iterator((4, Hash::default())) + .collect(); + assert_eq!( + parents, + vec![2] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() + ); } #[test] @@ -869,19 +1055,73 @@ mod test { .collect(); frozen_banks.sort_by_key(|bank| bank.slot()); - let root = bank_forks.read().unwrap().root(); + let root_bank = bank_forks.read().unwrap().root_bank(); + let root = root_bank.slot(); + let root_hash = root_bank.hash(); let heaviest_subtree_fork_choice = - HeaviestSubtreeForkChoice::new_from_frozen_banks(root, &frozen_banks); - assert!(heaviest_subtree_fork_choice.parent(0).is_none()); - assert_eq!(heaviest_subtree_fork_choice.children(0).unwrap(), &[1]); - assert_eq!(heaviest_subtree_fork_choice.parent(1), Some(0)); - assert_eq!(heaviest_subtree_fork_choice.children(1).unwrap(), &[2, 3]); - assert_eq!(heaviest_subtree_fork_choice.parent(2), Some(1)); - assert_eq!(heaviest_subtree_fork_choice.children(2).unwrap(), &[4]); - assert_eq!(heaviest_subtree_fork_choice.parent(3), Some(1)); - assert!(heaviest_subtree_fork_choice.children(3).unwrap().is_empty()); - assert_eq!(heaviest_subtree_fork_choice.parent(4), Some(2)); - assert!(heaviest_subtree_fork_choice.children(4).unwrap().is_empty()); + HeaviestSubtreeForkChoice::new_from_frozen_banks((root, root_hash), &frozen_banks); + + let bank0_hash = bank_forks.read().unwrap().get(0).unwrap().hash(); + assert!(heaviest_subtree_fork_choice + .parent(&(0, bank0_hash)) + .is_none()); + + let bank1_hash = bank_forks.read().unwrap().get(1).unwrap().hash(); + assert_eq!( + heaviest_subtree_fork_choice + .children(&(0, bank0_hash)) + .unwrap(), + &[(1, bank1_hash)] + ); + + assert_eq!( + heaviest_subtree_fork_choice.parent(&(1, bank1_hash)), + Some((0, bank0_hash)) + ); + let bank2_hash = bank_forks.read().unwrap().get(2).unwrap().hash(); + let bank3_hash = bank_forks.read().unwrap().get(3).unwrap().hash(); + assert_eq!( + heaviest_subtree_fork_choice + .children(&(1, bank1_hash)) + .unwrap(), + &[(2, bank2_hash), (3, bank3_hash)] + ); + assert_eq!( + heaviest_subtree_fork_choice.parent(&(2, bank2_hash)), + Some((1, bank1_hash)) + ); + let bank4_hash = bank_forks.read().unwrap().get(4).unwrap().hash(); + assert_eq!( + heaviest_subtree_fork_choice + .children(&(2, bank2_hash)) + .unwrap(), + &[(4, bank4_hash)] + ); + // Check parent and children of invalid hash don't exist + let invalid_hash = Hash::new_unique(); + assert!(heaviest_subtree_fork_choice + .children(&(2, invalid_hash)) + .is_none()); + assert!(heaviest_subtree_fork_choice + .parent(&(2, invalid_hash)) + .is_none()); + + assert_eq!( + heaviest_subtree_fork_choice.parent(&(3, bank3_hash)), + Some((1, bank1_hash)) + ); + assert!(heaviest_subtree_fork_choice + .children(&(3, bank3_hash)) + .unwrap() + .is_empty()); + assert_eq!( + heaviest_subtree_fork_choice.parent(&(4, bank4_hash)), + Some((2, bank2_hash)) + ); + assert!(heaviest_subtree_fork_choice + .children(&(4, bank4_hash)) + .unwrap() + .is_empty()); } #[test] @@ -889,21 +1129,25 @@ mod test { let mut heaviest_subtree_fork_choice = setup_forks(); // Set root to 1, should only purge 0 - heaviest_subtree_fork_choice.set_root(1); + heaviest_subtree_fork_choice.set_root((1, Hash::default())); for i in 0..=6 { let exists = i != 0; assert_eq!( - heaviest_subtree_fork_choice.fork_infos.contains_key(&i), + heaviest_subtree_fork_choice + .fork_infos + .contains_key(&(i, Hash::default())), exists ); } // Set root to 5, should purge everything except 5, 6 - heaviest_subtree_fork_choice.set_root(5); + heaviest_subtree_fork_choice.set_root((5, Hash::default())); for i in 0..=6 { let exists = i == 5 || i == 6; assert_eq!( - heaviest_subtree_fork_choice.fork_infos.contains_key(&i), + heaviest_subtree_fork_choice + .fork_infos + .contains_key(&(i, Hash::default())), exists ); } @@ -917,43 +1161,51 @@ mod test { // Vote for slot 2 heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 1)], + [(vote_pubkeys[0], (1, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 4); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 4); // Set a root - heaviest_subtree_fork_choice.set_root(1); + heaviest_subtree_fork_choice.set_root((1, Hash::default())); // Vote again for slot 3 on a different fork than the last vote, // verify this fork is now the best fork heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 3)], + [(vote_pubkeys[0], (3, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); - assert_eq!(heaviest_subtree_fork_choice.stake_voted_at(1).unwrap(), 0); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_at(3).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_at(&(1, Hash::default())) + .unwrap(), + 0 + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&(3, Hash::default())) + .unwrap(), stake ); for slot in &[1, 3] { assert_eq!( heaviest_subtree_fork_choice - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(), stake ); } // Set a root at last vote - heaviest_subtree_fork_choice.set_root(3); + heaviest_subtree_fork_choice.set_root((3, Hash::default())); // Check new leaf 7 is still propagated properly - heaviest_subtree_fork_choice.add_new_leaf_slot(7, Some(6)); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 7); + heaviest_subtree_fork_choice + .add_new_leaf_slot((7, Hash::default()), Some((6, Hash::default()))); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 7); } #[test] @@ -964,70 +1216,90 @@ mod test { // Vote for slot 0 heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 0)], + [(vote_pubkeys[0], (0, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); // Set root to 1, should purge 0 from the tree, but // there's still an outstanding vote for slot 0 in `pubkey_votes`. - heaviest_subtree_fork_choice.set_root(1); + heaviest_subtree_fork_choice.set_root((1, Hash::default())); // Vote again for slot 3, verify everything is ok heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 3)], + [(vote_pubkeys[0], (3, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_at(3).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_at(&(3, Hash::default())) + .unwrap(), stake ); for slot in &[1, 3] { assert_eq!( heaviest_subtree_fork_choice - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(), stake ); } - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); // Set root again on different fork than the last vote - heaviest_subtree_fork_choice.set_root(2); + heaviest_subtree_fork_choice.set_root((2, Hash::default())); // Smaller vote than last vote 3 should be ignored heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 2)], + [(vote_pubkeys[0], (2, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert_eq!(heaviest_subtree_fork_choice.stake_voted_at(2).unwrap(), 0); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_subtree(2).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_at(&(2, Hash::default())) + .unwrap(), 0 ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 4); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&(2, Hash::default())) + .unwrap(), + 0 + ); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 4); // New larger vote than last vote 3 should be processed heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 4)], + [(vote_pubkeys[0], (4, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert_eq!(heaviest_subtree_fork_choice.stake_voted_at(2).unwrap(), 0); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_at(4).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_at(&(2, Hash::default())) + .unwrap(), + 0 + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&(4, Hash::default())) + .unwrap(), stake ); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_subtree(2).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_subtree(&(2, Hash::default())) + .unwrap(), stake ); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_subtree(4).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_subtree(&(4, Hash::default())) + .unwrap(), stake ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 4); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 4); } #[test] @@ -1035,7 +1307,50 @@ mod test { let heaviest_subtree_fork_choice = setup_forks(); // Best overall path is 0 -> 1 -> 2 -> 4, so best leaf // should be 4 - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 4); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 4); + } + + #[test] + fn test_add_new_leaf_duplicate() { + let ( + mut heaviest_subtree_fork_choice, + duplicate_leaves_descended_from_4, + duplicate_leaves_descended_from_5, + ) = setup_duplicate_forks(); + + // Add a child to one of the duplicates + let duplicate_parent = duplicate_leaves_descended_from_4[0]; + let child = (11, Hash::new_unique()); + heaviest_subtree_fork_choice.add_new_leaf_slot(child, Some(duplicate_parent)); + assert_eq!( + heaviest_subtree_fork_choice + .children(&duplicate_parent) + .unwrap(), + &[child] + ); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), child); + + // All the other duplicates should have no children + for duplicate_leaf in duplicate_leaves_descended_from_5 + .iter() + .chain(std::iter::once(&duplicate_leaves_descended_from_4[1])) + { + assert!(heaviest_subtree_fork_choice + .children(&duplicate_leaf) + .unwrap() + .is_empty(),); + } + + // Re-adding same duplicate slot should not overwrite existing one + heaviest_subtree_fork_choice + .add_new_leaf_slot(duplicate_parent, Some((4, Hash::default()))); + assert_eq!( + heaviest_subtree_fork_choice + .children(&duplicate_parent) + .unwrap(), + &[child] + ); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), child); } #[test] @@ -1043,30 +1358,42 @@ mod test { let mut heaviest_subtree_fork_choice = setup_forks(); // Add a leaf 10, it should be the best choice - heaviest_subtree_fork_choice.add_new_leaf_slot(10, Some(4)); + heaviest_subtree_fork_choice + .add_new_leaf_slot((10, Hash::default()), Some((4, Hash::default()))); let ancestors = heaviest_subtree_fork_choice - .ancestor_iterator(10) + .ancestor_iterator((10, Hash::default())) .collect::>(); - for a in ancestors.into_iter().chain(std::iter::once(10)) { - assert_eq!(heaviest_subtree_fork_choice.best_slot(a).unwrap(), 10); + for a in ancestors + .into_iter() + .chain(std::iter::once((10, Hash::default()))) + { + assert_eq!(heaviest_subtree_fork_choice.best_slot(&a).unwrap().0, 10); } // Add a smaller leaf 9, it should be the best choice - heaviest_subtree_fork_choice.add_new_leaf_slot(9, Some(4)); + heaviest_subtree_fork_choice + .add_new_leaf_slot((9, Hash::default()), Some((4, Hash::default()))); let ancestors = heaviest_subtree_fork_choice - .ancestor_iterator(9) + .ancestor_iterator((9, Hash::default())) .collect::>(); - for a in ancestors.into_iter().chain(std::iter::once(9)) { - assert_eq!(heaviest_subtree_fork_choice.best_slot(a).unwrap(), 9); + for a in ancestors + .into_iter() + .chain(std::iter::once((9, Hash::default()))) + { + assert_eq!(heaviest_subtree_fork_choice.best_slot(&a).unwrap().0, 9); } // Add a higher leaf 11, should not change the best choice - heaviest_subtree_fork_choice.add_new_leaf_slot(11, Some(4)); + heaviest_subtree_fork_choice + .add_new_leaf_slot((11, Hash::default()), Some((4, Hash::default()))); let ancestors = heaviest_subtree_fork_choice - .ancestor_iterator(11) + .ancestor_iterator((11, Hash::default())) .collect::>(); - for a in ancestors.into_iter().chain(std::iter::once(9)) { - assert_eq!(heaviest_subtree_fork_choice.best_slot(a).unwrap(), 9); + for a in ancestors + .into_iter() + .chain(std::iter::once((9, Hash::default()))) + { + assert_eq!(heaviest_subtree_fork_choice.best_slot(&a).unwrap().0, 9); } // Add a vote for the other branch at slot 3. @@ -1076,7 +1403,7 @@ mod test { // Leaf slot 9 stops being the `best_slot` at slot 1 because there // are now votes for the branch at slot 3 heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], leaf6)], + [(vote_pubkeys[0], (leaf6, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); @@ -1084,14 +1411,18 @@ mod test { // Because slot 1 now sees the child branch at slot 3 has non-zero // weight, adding smaller leaf slot 8 in the other child branch at slot 2 // should not propagate past slot 1 - heaviest_subtree_fork_choice.add_new_leaf_slot(8, Some(4)); + heaviest_subtree_fork_choice + .add_new_leaf_slot((8, Hash::default()), Some((4, Hash::default()))); let ancestors = heaviest_subtree_fork_choice - .ancestor_iterator(8) + .ancestor_iterator((8, Hash::default())) .collect::>(); - for a in ancestors.into_iter().chain(std::iter::once(8)) { - let best_slot = if a > 1 { 8 } else { leaf6 }; + for a in ancestors + .into_iter() + .chain(std::iter::once((8, Hash::default()))) + { + let best_slot = if a.0 > 1 { 8 } else { leaf6 }; assert_eq!( - heaviest_subtree_fork_choice.best_slot(a).unwrap(), + heaviest_subtree_fork_choice.best_slot(&a).unwrap().0, best_slot ); } @@ -1099,26 +1430,33 @@ mod test { // Add vote for slot 8, should now be the best slot (has same weight // as fork containing slot 6, but slot 2 is smaller than slot 3). heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[1], 8)], + [(vote_pubkeys[1], (8, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 8); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 8); // Because slot 4 now sees the child leaf 8 has non-zero // weight, adding smaller leaf slots should not propagate past slot 4 - heaviest_subtree_fork_choice.add_new_leaf_slot(7, Some(4)); + heaviest_subtree_fork_choice + .add_new_leaf_slot((7, Hash::default()), Some((4, Hash::default()))); let ancestors = heaviest_subtree_fork_choice - .ancestor_iterator(7) + .ancestor_iterator((7, Hash::default())) .collect::>(); - for a in ancestors.into_iter().chain(std::iter::once(8)) { - assert_eq!(heaviest_subtree_fork_choice.best_slot(a).unwrap(), 8); + for a in ancestors + .into_iter() + .chain(std::iter::once((8, Hash::default()))) + { + assert_eq!(heaviest_subtree_fork_choice.best_slot(&a).unwrap().0, 8); } // All the leaves should think they are their own best choice for leaf in [8, 9, 10, 11].iter() { assert_eq!( - heaviest_subtree_fork_choice.best_slot(*leaf).unwrap(), + heaviest_subtree_fork_choice + .best_slot(&(*leaf, Hash::default())) + .unwrap() + .0, *leaf ); } @@ -1140,31 +1478,34 @@ mod test { let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(1, stake); // slot 6 should be the best because it's the only leaf - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); // Add a leaf slot 5. Even though 5 is less than the best leaf 6, // it's not less than it's sibling slot 4, so the best overall // leaf should remain unchanged - heaviest_subtree_fork_choice.add_new_leaf_slot(5, Some(0)); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); + heaviest_subtree_fork_choice + .add_new_leaf_slot((5, Hash::default()), Some((0, Hash::default()))); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); // Add a leaf slot 2 on a different fork than leaf 6. Slot 2 should // be the new best because it's for a lesser slot - heaviest_subtree_fork_choice.add_new_leaf_slot(2, Some(0)); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 2); + heaviest_subtree_fork_choice + .add_new_leaf_slot((2, Hash::default()), Some((0, Hash::default()))); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 2); // Add a vote for slot 4, so leaf 6 should be the best again heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 4)], + [(vote_pubkeys[0], (4, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); // Adding a slot 1 that is less than the current best leaf 6 should not change the best // slot because the fork slot 5 is on has a higher weight - heaviest_subtree_fork_choice.add_new_leaf_slot(1, Some(0)); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); + heaviest_subtree_fork_choice + .add_new_leaf_slot((1, Hash::default()), Some((0, Hash::default()))); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); } #[test] @@ -1172,16 +1513,41 @@ mod test { let mut heaviest_subtree_fork_choice = setup_forks(); // No weights are present, weights should be zero - heaviest_subtree_fork_choice.aggregate_slot(1); - assert_eq!(heaviest_subtree_fork_choice.stake_voted_at(1).unwrap(), 0); + heaviest_subtree_fork_choice.aggregate_slot((1, Hash::default())); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_subtree(1).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_at(&(1, Hash::default())) + .unwrap(), + 0 + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&(1, Hash::default())) + .unwrap(), 0 ); // The best leaf when weights are equal should prioritize the lower leaf - assert_eq!(heaviest_subtree_fork_choice.best_slot(1).unwrap(), 4); - assert_eq!(heaviest_subtree_fork_choice.best_slot(2).unwrap(), 4); - assert_eq!(heaviest_subtree_fork_choice.best_slot(3).unwrap(), 6); + assert_eq!( + heaviest_subtree_fork_choice + .best_slot(&(1, Hash::default())) + .unwrap() + .0, + 4 + ); + assert_eq!( + heaviest_subtree_fork_choice + .best_slot(&(2, Hash::default())) + .unwrap() + .0, + 4 + ); + assert_eq!( + heaviest_subtree_fork_choice + .best_slot(&(3, Hash::default())) + .unwrap() + .0, + 6 + ); // Update the weights that have voted *exactly* at each slot, the // branch containing slots {5, 6} has weight 11, so should be heavier @@ -1189,26 +1555,26 @@ mod test { let mut total_stake = 0; let staked_voted_slots: HashSet<_> = vec![2, 4, 5, 6].into_iter().collect(); for slot in &staked_voted_slots { - heaviest_subtree_fork_choice.set_stake_voted_at(*slot, *slot); + heaviest_subtree_fork_choice.set_stake_voted_at((*slot, Hash::default()), *slot); total_stake += *slot; } // Aggregate up each of the two forks (order matters, has to be // reverse order for each fork, and aggregating a slot multiple times // is fine) - let slots_to_aggregate: Vec<_> = std::iter::once(6) - .chain(heaviest_subtree_fork_choice.ancestor_iterator(6)) - .chain(std::iter::once(4)) - .chain(heaviest_subtree_fork_choice.ancestor_iterator(4)) + let slots_to_aggregate: Vec<_> = std::iter::once((6, Hash::default())) + .chain(heaviest_subtree_fork_choice.ancestor_iterator((6, Hash::default()))) + .chain(std::iter::once((4, Hash::default()))) + .chain(heaviest_subtree_fork_choice.ancestor_iterator((4, Hash::default()))) .collect(); - for slot in slots_to_aggregate { - heaviest_subtree_fork_choice.aggregate_slot(slot); + for slot_hash in slots_to_aggregate { + heaviest_subtree_fork_choice.aggregate_slot(slot_hash); } // The best path is now 0 -> 1 -> 3 -> 5 -> 6, so leaf 6 // should be the best choice - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 6); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 6); // Verify `stake_voted_at` for slot in 0..=6 { @@ -1219,7 +1585,9 @@ mod test { }; assert_eq!( - heaviest_subtree_fork_choice.stake_voted_at(slot).unwrap(), + heaviest_subtree_fork_choice + .stake_voted_at(&(slot, Hash::default())) + .unwrap(), expected_stake ); } @@ -1230,7 +1598,7 @@ mod test { // all slots in the subtree assert_eq!( heaviest_subtree_fork_choice - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(), total_stake ); @@ -1238,10 +1606,12 @@ mod test { // Verify `stake_voted_subtree` for fork 1 let mut total_expected_stake = 0; for slot in &[4, 2] { - total_expected_stake += heaviest_subtree_fork_choice.stake_voted_at(*slot).unwrap(); + total_expected_stake += heaviest_subtree_fork_choice + .stake_voted_at(&(*slot, Hash::default())) + .unwrap(); assert_eq!( heaviest_subtree_fork_choice - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(), total_expected_stake ); @@ -1249,10 +1619,12 @@ mod test { // Verify `stake_voted_subtree` for fork 2 total_expected_stake = 0; for slot in &[6, 5, 3] { - total_expected_stake += heaviest_subtree_fork_choice.stake_voted_at(*slot).unwrap(); + total_expected_stake += heaviest_subtree_fork_choice + .stake_voted_at(&(*slot, Hash::default())) + .unwrap(); assert_eq!( heaviest_subtree_fork_choice - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(), total_expected_stake ); @@ -1265,19 +1637,19 @@ mod test { let stake = 100; let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, stake); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 3), - (vote_pubkeys[1], 2), - (vote_pubkeys[2], 1), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (3, Hash::default())), + (vote_pubkeys[1], (2, Hash::default())), + (vote_pubkeys[2], (1, Hash::default())), ]; let expected_best_slot = |slot, heaviest_subtree_fork_choice: &HeaviestSubtreeForkChoice| -> Slot { - if !heaviest_subtree_fork_choice.is_leaf(slot) { + if !heaviest_subtree_fork_choice.is_leaf((slot, Hash::default())) { // Both branches have equal weight, so should pick the lesser leaf if heaviest_subtree_fork_choice - .ancestor_iterator(4) - .collect::>() - .contains(&slot) + .ancestor_iterator((4, Hash::default())) + .collect::>() + .contains(&(slot, Hash::default())) { 4 } else { @@ -1298,20 +1670,20 @@ mod test { ); // Everyone makes newer votes - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 4), - (vote_pubkeys[1], 3), - (vote_pubkeys[2], 3), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (4, Hash::default())), + (vote_pubkeys[1], (3, Hash::default())), + (vote_pubkeys[2], (3, Hash::default())), ]; let expected_best_slot = |slot, heaviest_subtree_fork_choice: &HeaviestSubtreeForkChoice| -> Slot { - if !heaviest_subtree_fork_choice.is_leaf(slot) { + if !heaviest_subtree_fork_choice.is_leaf((slot, Hash::default())) { // The branch with leaf 6 now has two votes, so should pick that one if heaviest_subtree_fork_choice - .ancestor_iterator(6) - .collect::>() - .contains(&slot) + .ancestor_iterator((6, Hash::default())) + .collect::>() + .contains(&(slot, Hash::default())) { 6 } else { @@ -1337,108 +1709,176 @@ mod test { let mut heaviest_subtree_fork_choice = setup_forks(); let stake = 100; let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, stake); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 3), - (vote_pubkeys[1], 4), - (vote_pubkeys[2], 1), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (3, Hash::default())), + (vote_pubkeys[1], (4, Hash::default())), + (vote_pubkeys[2], (1, Hash::default())), ]; - let expected_update_operations: BTreeMap<(Slot, UpdateLabel), UpdateOperation> = vec![ + let expected_update_operations: UpdateOperations = vec![ // Add/remove from new/old forks - ((1, UpdateLabel::Add), UpdateOperation::Add(stake)), - ((3, UpdateLabel::Add), UpdateOperation::Add(stake)), - ((4, UpdateLabel::Add), UpdateOperation::Add(stake)), + ( + ((1, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(stake), + ), + ( + ((3, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(stake), + ), + ( + ((4, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(stake), + ), // Aggregate all ancestors of changed slots - ((0, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((1, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((2, UpdateLabel::Aggregate), UpdateOperation::Aggregate), + ( + ((0, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((1, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((2, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), ] .into_iter() .collect(); let generated_update_operations = heaviest_subtree_fork_choice.generate_update_operations( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); assert_eq!(expected_update_operations, generated_update_operations); // Everyone makes older/same votes, should be ignored - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 3), - (vote_pubkeys[1], 2), - (vote_pubkeys[2], 1), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (3, Hash::default())), + (vote_pubkeys[1], (2, Hash::default())), + (vote_pubkeys[2], (1, Hash::default())), ]; let generated_update_operations = heaviest_subtree_fork_choice.generate_update_operations( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); assert!(generated_update_operations.is_empty()); // Some people make newer votes - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ // old, ignored - (vote_pubkeys[0], 3), + (vote_pubkeys[0], (3, Hash::default())), // new, switched forks - (vote_pubkeys[1], 5), + (vote_pubkeys[1], (5, Hash::default())), // new, same fork - (vote_pubkeys[2], 3), + (vote_pubkeys[2], (3, Hash::default())), ]; - let expected_update_operations: BTreeMap<(Slot, UpdateLabel), UpdateOperation> = vec![ - // Add/remove to/from new/old forks - ((3, UpdateLabel::Add), UpdateOperation::Add(stake)), - ((5, UpdateLabel::Add), UpdateOperation::Add(stake)), - ((1, UpdateLabel::Subtract), UpdateOperation::Subtract(stake)), - ((4, UpdateLabel::Subtract), UpdateOperation::Subtract(stake)), - // Aggregate all ancestors of changed slots - ((0, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((1, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((2, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((3, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ] - .into_iter() - .collect(); + let expected_update_operations: BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation> = + vec![ + // Add/remove to/from new/old forks + ( + ((3, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(stake), + ), + ( + ((5, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(stake), + ), + ( + ((1, Hash::default()), UpdateLabel::Subtract), + UpdateOperation::Subtract(stake), + ), + ( + ((4, Hash::default()), UpdateLabel::Subtract), + UpdateOperation::Subtract(stake), + ), + // Aggregate all ancestors of changed slots + ( + ((0, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((1, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((2, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((3, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ] + .into_iter() + .collect(); let generated_update_operations = heaviest_subtree_fork_choice.generate_update_operations( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); assert_eq!(expected_update_operations, generated_update_operations); // People make new votes - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ // new, switch forks - (vote_pubkeys[0], 4), + (vote_pubkeys[0], (4, Hash::default())), // new, same fork - (vote_pubkeys[1], 6), + (vote_pubkeys[1], (6, Hash::default())), // new, same fork - (vote_pubkeys[2], 6), + (vote_pubkeys[2], (6, Hash::default())), ]; - let expected_update_operations: BTreeMap<(Slot, UpdateLabel), UpdateOperation> = vec![ - // Add/remove from new/old forks - ((4, UpdateLabel::Add), UpdateOperation::Add(stake)), - ((6, UpdateLabel::Add), UpdateOperation::Add(2 * stake)), - ( - (3, UpdateLabel::Subtract), - UpdateOperation::Subtract(2 * stake), - ), - ((5, UpdateLabel::Subtract), UpdateOperation::Subtract(stake)), - // Aggregate all ancestors of changed slots - ((0, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((1, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((2, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((3, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ((5, UpdateLabel::Aggregate), UpdateOperation::Aggregate), - ] - .into_iter() - .collect(); + let expected_update_operations: BTreeMap<(SlotHashKey, UpdateLabel), UpdateOperation> = + vec![ + // Add/remove from new/old forks + ( + ((4, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(stake), + ), + ( + ((6, Hash::default()), UpdateLabel::Add), + UpdateOperation::Add(2 * stake), + ), + ( + ((3, Hash::default()), UpdateLabel::Subtract), + UpdateOperation::Subtract(2 * stake), + ), + ( + ((5, Hash::default()), UpdateLabel::Subtract), + UpdateOperation::Subtract(stake), + ), + // Aggregate all ancestors of changed slots + ( + ((0, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((1, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((2, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((3, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ( + ((5, Hash::default()), UpdateLabel::Aggregate), + UpdateOperation::Aggregate, + ), + ] + .into_iter() + .collect(); let generated_update_operations = heaviest_subtree_fork_choice.generate_update_operations( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); @@ -1451,21 +1891,471 @@ mod test { let stake = 100; let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, stake); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 3), - (vote_pubkeys[1], 2), - (vote_pubkeys[2], 1), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (3, Hash::default())), + (vote_pubkeys[1], (2, Hash::default())), + (vote_pubkeys[2], (1, Hash::default())), ]; assert_eq!( - heaviest_subtree_fork_choice.add_votes( - &pubkey_votes, - bank.epoch_stakes_map(), - bank.epoch_schedule() - ), + heaviest_subtree_fork_choice + .add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ) + .0, 4 ); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 4) + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 4) + } + + #[test] + fn test_add_votes_duplicate_tie() { + let (mut heaviest_subtree_fork_choice, duplicate_leaves_descended_from_4, _) = + setup_duplicate_forks(); + let stake = 10; + let num_validators = 2; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(num_validators, stake); + + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], duplicate_leaves_descended_from_4[0]), + (vote_pubkeys[1], duplicate_leaves_descended_from_4[1]), + ]; + + // duplicate_leaves_descended_from_4 are sorted, and fork choice will pick the smaller + // one in the event of a tie + let expected_best_slot_hash = duplicate_leaves_descended_from_4[0]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + expected_best_slot_hash + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake + ); + + // Adding the same vote again will not do anything + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = + vec![(vote_pubkeys[1], duplicate_leaves_descended_from_4[1])]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake + ); + + // All common ancestors should have subtree voted stake == 2 * stake, but direct + // voted stake == 0 + let expected_ancestors_stake = 2 * stake; + for ancestor in + heaviest_subtree_fork_choice.ancestor_iterator(duplicate_leaves_descended_from_4[1]) + { + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&ancestor) + .unwrap(), + expected_ancestors_stake + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&ancestor) + .unwrap(), + 0, + ); + } + } + + #[test] + fn test_add_votes_duplicate_greater_hash_ignored() { + let (mut heaviest_subtree_fork_choice, duplicate_leaves_descended_from_4, _) = + setup_duplicate_forks(); + let stake = 10; + let num_validators = 2; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(num_validators, stake); + + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], duplicate_leaves_descended_from_4[0]), + (vote_pubkeys[1], duplicate_leaves_descended_from_4[1]), + ]; + + // duplicate_leaves_descended_from_4 are sorted, and fork choice will pick the smaller + // one in the event of a tie + let expected_best_slot_hash = duplicate_leaves_descended_from_4[0]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + // Adding a duplicate vote for a validator, for another a greater bank hash, + // should be ignored as we prioritize the smaller bank hash. Thus nothing + // should change. + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = + vec![(vote_pubkeys[0], duplicate_leaves_descended_from_4[1])]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + // Still only has one validator voting on it + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake + ); + // All common ancestors should have subtree voted stake == 2 * stake, but direct + // voted stake == 0 + let expected_ancestors_stake = 2 * stake; + for ancestor in + heaviest_subtree_fork_choice.ancestor_iterator(duplicate_leaves_descended_from_4[1]) + { + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&ancestor) + .unwrap(), + expected_ancestors_stake + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&ancestor) + .unwrap(), + 0, + ); + } + } + + #[test] + fn test_add_votes_duplicate_smaller_hash_prioritized() { + let (mut heaviest_subtree_fork_choice, duplicate_leaves_descended_from_4, _) = + setup_duplicate_forks(); + let stake = 10; + let num_validators = 2; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(num_validators, stake); + + // Both voters voted on duplicate_leaves_descended_from_4[1], so thats the heaviest + // branch + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], duplicate_leaves_descended_from_4[1]), + (vote_pubkeys[1], duplicate_leaves_descended_from_4[1]), + ]; + + let expected_best_slot_hash = duplicate_leaves_descended_from_4[1]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + // BEFORE, both validators voting on this leaf + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + 2 * stake, + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + 2 * stake, + ); + + // Adding a duplicate vote for a validator, for another a smaller bank hash, + // should be proritized and replace the vote for the greater bank hash. + // Now because both duplicate nodes are tied, the best leaf is the smaller one. + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = + vec![(vote_pubkeys[0], duplicate_leaves_descended_from_4[0])]; + let expected_best_slot_hash = duplicate_leaves_descended_from_4[0]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + // AFTER, only one of the validators is voting on this leaf + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake, + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&duplicate_leaves_descended_from_4[1]) + .unwrap(), + stake, + ); + + // The other leaf now has one of the votes + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&duplicate_leaves_descended_from_4[0]) + .unwrap(), + stake, + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&duplicate_leaves_descended_from_4[0]) + .unwrap(), + stake, + ); + + // All common ancestors should have subtree voted stake == 2 * stake, but direct + // voted stake == 0 + let expected_ancestors_stake = 2 * stake; + for ancestor in + heaviest_subtree_fork_choice.ancestor_iterator(duplicate_leaves_descended_from_4[0]) + { + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&ancestor) + .unwrap(), + expected_ancestors_stake + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&ancestor) + .unwrap(), + 0, + ); + } + } + + #[test] + fn test_add_votes_duplicate_then_outdated() { + let (mut heaviest_subtree_fork_choice, duplicate_leaves_descended_from_4, _) = + setup_duplicate_forks(); + let stake = 10; + let num_validators = 3; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(num_validators, stake); + + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], duplicate_leaves_descended_from_4[0]), + (vote_pubkeys[1], duplicate_leaves_descended_from_4[1]), + ]; + + // duplicate_leaves_descended_from_4 are sorted, and fork choice will pick the smaller + // one in the event of a tie + let expected_best_slot_hash = duplicate_leaves_descended_from_4[0]; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + // Create two children for slots greater than the duplicate slot, + // 1) descended from the current best slot (which also happens to be a duplicate slot) + // 2) another descended from a non-duplicate slot. + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + duplicate_leaves_descended_from_4[0] + ); + // Create new child with heaviest duplicate parent + let duplicate_parent = duplicate_leaves_descended_from_4[0]; + let duplicate_slot = duplicate_parent.0; + + // Create new child with non-duplicate parent + let nonduplicate_parent = (2, Hash::default()); + let higher_child_with_duplicate_parent = (duplicate_slot + 1, Hash::new_unique()); + let higher_child_with_nonduplicate_parent = (duplicate_slot + 2, Hash::new_unique()); + heaviest_subtree_fork_choice + .add_new_leaf_slot(higher_child_with_duplicate_parent, Some(duplicate_parent)); + heaviest_subtree_fork_choice.add_new_leaf_slot( + higher_child_with_nonduplicate_parent, + Some(nonduplicate_parent), + ); + + // vote_pubkeys[0] and vote_pubkeys[1] should both have their latest votes + // erased after a vote for a higher parent + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], higher_child_with_duplicate_parent), + (vote_pubkeys[1], higher_child_with_nonduplicate_parent), + (vote_pubkeys[2], higher_child_with_nonduplicate_parent), + ]; + let expected_best_slot_hash = higher_child_with_nonduplicate_parent; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + + // All the stake dirctly voting on the duplicates have been outdated + for (i, duplicate_leaf) in duplicate_leaves_descended_from_4.iter().enumerate() { + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(duplicate_leaf) + .unwrap(), + 0, + ); + + if i == 0 { + // The subtree stake of the first duplicate however, has one vote still + // because it's the parent of the `higher_child_with_duplicate_parent`, + // which has one vote + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(duplicate_leaf) + .unwrap(), + stake, + ); + } else { + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(duplicate_leaf) + .unwrap(), + 0, + ); + } + } + + // Node 4 has subtree voted stake == stake since it only has one voter on it + let node4 = (4, Hash::default()); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&node4) + .unwrap(), + stake, + ); + assert_eq!( + heaviest_subtree_fork_choice.stake_voted_at(&node4).unwrap(), + 0, + ); + + // All ancestors of 4 should have subtree voted stake == num_validators * stake, + // but direct voted stake == 0 + let expected_ancestors_stake = num_validators as u64 * stake; + for ancestor in heaviest_subtree_fork_choice.ancestor_iterator(node4) { + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_subtree(&ancestor) + .unwrap(), + expected_ancestors_stake + ); + assert_eq!( + heaviest_subtree_fork_choice + .stake_voted_at(&ancestor) + .unwrap(), + 0, + ); + } + } + + #[test] + fn test_add_votes_duplicate_zero_stake() { + let (mut heaviest_subtree_fork_choice, duplicate_leaves_descended_from_4, _): ( + HeaviestSubtreeForkChoice, + Vec, + Vec, + ) = setup_duplicate_forks(); + + let stake = 0; + let num_validators = 2; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(num_validators, stake); + + // Make new vote with vote_pubkeys[0] for a higher slot + // Create new child with heaviest duplicate parent + let duplicate_parent = duplicate_leaves_descended_from_4[0]; + let duplicate_slot = duplicate_parent.0; + let higher_child_with_duplicate_parent = (duplicate_slot + 1, Hash::new_unique()); + heaviest_subtree_fork_choice + .add_new_leaf_slot(higher_child_with_duplicate_parent, Some(duplicate_parent)); + + // Vote for pubkey 0 on one of the duplicate slots + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = + vec![(vote_pubkeys[0], duplicate_leaves_descended_from_4[1])]; + + // Stake is zero, so because duplicate_leaves_descended_from_4[0] and + // duplicate_leaves_descended_from_4[1] are tied, the child of the smaller + // node duplicate_leaves_descended_from_4[0] is the one that is picked + let expected_best_slot_hash = higher_child_with_duplicate_parent; + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + assert_eq!( + *heaviest_subtree_fork_choice + .latest_votes + .get(&vote_pubkeys[0]) + .unwrap(), + duplicate_leaves_descended_from_4[1] + ); + + // Now add a vote for a higher slot, and ensure the latest votes + // for this pubkey were updated + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = + vec![(vote_pubkeys[0], higher_child_with_duplicate_parent)]; + + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule() + ), + expected_best_slot_hash + ); + assert_eq!( + *heaviest_subtree_fork_choice + .latest_votes + .get(&vote_pubkeys[0]) + .unwrap(), + higher_child_with_duplicate_parent + ); } #[test] @@ -1480,35 +2370,36 @@ mod test { */ let forks = tr(0) / (tr(4) / (tr(9)) / (tr(10))); let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new_from_tree(forks); - assert!(heaviest_subtree_fork_choice.is_best_child(0)); - assert!(heaviest_subtree_fork_choice.is_best_child(4)); + assert!(heaviest_subtree_fork_choice.is_best_child(&(0, Hash::default()))); + assert!(heaviest_subtree_fork_choice.is_best_child(&(4, Hash::default()))); // 9 is better than 10 - assert!(heaviest_subtree_fork_choice.is_best_child(9)); - assert!(!heaviest_subtree_fork_choice.is_best_child(10)); + assert!(heaviest_subtree_fork_choice.is_best_child(&(9, Hash::default()))); + assert!(!heaviest_subtree_fork_choice.is_best_child(&(10, Hash::default()))); // Add new leaf 8, which is better than 9, as both have weight 0 - heaviest_subtree_fork_choice.add_new_leaf_slot(8, Some(4)); - assert!(heaviest_subtree_fork_choice.is_best_child(8)); - assert!(!heaviest_subtree_fork_choice.is_best_child(9)); - assert!(!heaviest_subtree_fork_choice.is_best_child(10)); + heaviest_subtree_fork_choice + .add_new_leaf_slot((8, Hash::default()), Some((4, Hash::default()))); + assert!(heaviest_subtree_fork_choice.is_best_child(&(8, Hash::default()))); + assert!(!heaviest_subtree_fork_choice.is_best_child(&(9, Hash::default()))); + assert!(!heaviest_subtree_fork_choice.is_best_child(&(10, Hash::default()))); // Add vote for 9, it's the best again let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, 100); heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 9)], + [(vote_pubkeys[0], (9, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - assert!(heaviest_subtree_fork_choice.is_best_child(9)); - assert!(!heaviest_subtree_fork_choice.is_best_child(8)); - assert!(!heaviest_subtree_fork_choice.is_best_child(10)); + assert!(heaviest_subtree_fork_choice.is_best_child(&(9, Hash::default()))); + assert!(!heaviest_subtree_fork_choice.is_best_child(&(8, Hash::default()))); + assert!(!heaviest_subtree_fork_choice.is_best_child(&(10, Hash::default()))); } #[test] fn test_merge() { let stake = 100; - let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, stake); + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(4, stake); /* Build fork structure: slot 0 @@ -1524,13 +2415,13 @@ mod test { */ let forks = tr(0) / (tr(3) / (tr(5) / (tr(7))) / (tr(9) / (tr(11) / (tr(12))))); let mut tree1 = HeaviestSubtreeForkChoice::new_from_tree(forks); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 5), - (vote_pubkeys[1], 3), - (vote_pubkeys[2], 12), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (5, Hash::default())), + (vote_pubkeys[1], (3, Hash::default())), + (vote_pubkeys[2], (12, Hash::default())), ]; tree1.add_votes( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); @@ -1546,58 +2437,193 @@ mod test { slot 17 | slot 19 (vote pubkey 1) | - slot 20 - */ + slot 20 (vote pubkey 3) + */ let forks = tr(10) / (tr(15) / (tr(16) / (tr(17))) / (tr(18) / (tr(19) / (tr(20))))); let mut tree2 = HeaviestSubtreeForkChoice::new_from_tree(forks); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ // more than tree 1 - (vote_pubkeys[0], 16), + (vote_pubkeys[0], (16, Hash::default())), // more than tree1 - (vote_pubkeys[1], 19), + (vote_pubkeys[1], (19, Hash::default())), // less than tree1 - (vote_pubkeys[2], 10), + (vote_pubkeys[2], (10, Hash::default())), + // Add a pubkey that only voted on this tree + (vote_pubkeys[3], (20, Hash::default())), ]; tree2.add_votes( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); // Merge tree2 at leaf 7 of tree1 - tree1.merge(tree2, 7, bank.epoch_stakes_map(), bank.epoch_schedule()); + tree1.merge( + tree2, + &(7, Hash::default()), + bank.epoch_stakes_map(), + bank.epoch_schedule(), + ); // Check ancestry information is correct - let ancestors: Vec<_> = tree1.ancestor_iterator(20).collect(); - assert_eq!(ancestors, vec![19, 18, 15, 10, 7, 5, 3, 0]); - let ancestors: Vec<_> = tree1.ancestor_iterator(17).collect(); - assert_eq!(ancestors, vec![16, 15, 10, 7, 5, 3, 0]); + let ancestors: Vec<_> = tree1.ancestor_iterator((20, Hash::default())).collect(); + assert_eq!( + ancestors, + vec![19, 18, 15, 10, 7, 5, 3, 0] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() + ); + let ancestors: Vec<_> = tree1.ancestor_iterator((17, Hash::default())).collect(); + assert_eq!( + ancestors, + vec![16, 15, 10, 7, 5, 3, 0] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() + ); - // Check correctness off votes + // Check correctness of votes // Pubkey 0 - assert_eq!(tree1.stake_voted_at(16).unwrap(), stake); - assert_eq!(tree1.stake_voted_at(5).unwrap(), 0); + assert_eq!(tree1.stake_voted_at(&(16, Hash::default())).unwrap(), stake); + assert_eq!(tree1.stake_voted_at(&(5, Hash::default())).unwrap(), 0); // Pubkey 1 - assert_eq!(tree1.stake_voted_at(19).unwrap(), stake); - assert_eq!(tree1.stake_voted_at(3).unwrap(), 0); + assert_eq!(tree1.stake_voted_at(&(19, Hash::default())).unwrap(), stake); + assert_eq!(tree1.stake_voted_at(&(3, Hash::default())).unwrap(), 0); // Pubkey 2 - assert_eq!(tree1.stake_voted_at(10).unwrap(), 0); - assert_eq!(tree1.stake_voted_at(12).unwrap(), stake); + assert_eq!(tree1.stake_voted_at(&(10, Hash::default())).unwrap(), 0); + assert_eq!(tree1.stake_voted_at(&(12, Hash::default())).unwrap(), stake); + // Pubkey 3 + assert_eq!(tree1.stake_voted_at(&(20, Hash::default())).unwrap(), stake); for slot in &[0, 3] { - assert_eq!(tree1.stake_voted_subtree(*slot).unwrap(), 3 * stake); + assert_eq!( + tree1 + .stake_voted_subtree(&(*slot, Hash::default())) + .unwrap(), + 4 * stake + ); } for slot in &[5, 7, 10, 15] { - assert_eq!(tree1.stake_voted_subtree(*slot).unwrap(), 2 * stake); + assert_eq!( + tree1 + .stake_voted_subtree(&(*slot, Hash::default())) + .unwrap(), + 3 * stake + ); } - for slot in &[9, 11, 12, 16, 18, 19] { - assert_eq!(tree1.stake_voted_subtree(*slot).unwrap(), stake); + for slot in &[18, 19] { + assert_eq!( + tree1 + .stake_voted_subtree(&(*slot, Hash::default())) + .unwrap(), + 2 * stake + ); } - for slot in &[17, 20] { - assert_eq!(tree1.stake_voted_subtree(*slot).unwrap(), 0); + for slot in &[9, 11, 12, 16, 20] { + assert_eq!( + tree1 + .stake_voted_subtree(&(*slot, Hash::default())) + .unwrap(), + stake + ); + } + for slot in &[17] { + assert_eq!( + tree1 + .stake_voted_subtree(&(*slot, Hash::default())) + .unwrap(), + 0 + ); } - assert_eq!(tree1.best_overall_slot(), 17); + assert_eq!(tree1.best_overall_slot().0, 20); + } + + #[test] + fn test_merge_duplicate() { + let stake = 100; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(2, stake); + let mut slot_5_duplicate_hashes = std::iter::repeat_with(|| (5, Hash::new_unique())) + .take(2) + .collect::>(); + slot_5_duplicate_hashes.sort(); + + /* + Build fork structure: + slot 0 + / \ + slot 2 slot 5 (bigger hash) + */ + let forks = + tr((0, Hash::default())) / tr((2, Hash::default())) / tr(slot_5_duplicate_hashes[1]); + let mut tree1 = HeaviestSubtreeForkChoice::new_from_tree(forks); + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (2, Hash::default())), + (vote_pubkeys[1], slot_5_duplicate_hashes[1]), + ]; + tree1.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule(), + ); + + /* + Build fork structure: + slot 3 + | + slot 5 (smaller hash, prioritized over previous version) + */ + let forks = tr((3, Hash::default())) / tr(slot_5_duplicate_hashes[0]); + let mut tree2 = HeaviestSubtreeForkChoice::new_from_tree(forks); + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (3, Hash::default())), + // Pubkey 1 voted on another version of slot 5 + (vote_pubkeys[1], slot_5_duplicate_hashes[0]), + ]; + + tree2.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule(), + ); + + // Merge tree2 at leaf 2 of tree1 + tree1.merge( + tree2, + &(2, Hash::default()), + bank.epoch_stakes_map(), + bank.epoch_schedule(), + ); + + // Pubkey 1 voted on both versions of slot 5, but should prioritize the one in + // the merged branch because it's for a smaller hash + assert_eq!( + tree1.stake_voted_at(&slot_5_duplicate_hashes[1]).unwrap(), + 0 + ); + assert_eq!( + tree1.stake_voted_at(&slot_5_duplicate_hashes[0]).unwrap(), + stake + ); + assert_eq!(tree1.best_overall_slot(), slot_5_duplicate_hashes[0]); + + // Check the ancestors are correct + let ancestors: Vec<_> = tree1 + .ancestor_iterator(slot_5_duplicate_hashes[1]) + .collect(); + assert_eq!(ancestors, vec![(0, Hash::default())]); + let ancestors: Vec<_> = tree1 + .ancestor_iterator(slot_5_duplicate_hashes[0]) + .collect(); + assert_eq!( + ancestors, + vec![ + (3, Hash::default()), + (2, Hash::default()), + (0, Hash::default()) + ] + ); } #[test] @@ -1605,36 +2631,53 @@ mod test { let mut heaviest_subtree_fork_choice = setup_forks(); // Diff of same root is empty, no matter root, intermediate node, or leaf - assert!(heaviest_subtree_fork_choice.subtree_diff(0, 0).is_empty()); - assert!(heaviest_subtree_fork_choice.subtree_diff(5, 5).is_empty()); - assert!(heaviest_subtree_fork_choice.subtree_diff(6, 6).is_empty()); + assert!(heaviest_subtree_fork_choice + .subtree_diff((0, Hash::default()), (0, Hash::default())) + .is_empty()); + assert!(heaviest_subtree_fork_choice + .subtree_diff((5, Hash::default()), (5, Hash::default())) + .is_empty()); + assert!(heaviest_subtree_fork_choice + .subtree_diff((6, Hash::default()), (6, Hash::default())) + .is_empty()); // The set reachable from slot 3, excluding subtree 1, is just everything // in slot 3 since subtree 1 is an ancestor assert_eq!( - heaviest_subtree_fork_choice.subtree_diff(3, 1), - vec![3, 5, 6].into_iter().collect::>() + heaviest_subtree_fork_choice.subtree_diff((3, Hash::default()), (1, Hash::default())), + vec![3, 5, 6] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() ); // The set reachable from slot 1, excluding subtree 3, is just 1 and // the subtree at 2 assert_eq!( - heaviest_subtree_fork_choice.subtree_diff(1, 3), - vec![1, 2, 4].into_iter().collect::>() + heaviest_subtree_fork_choice.subtree_diff((1, Hash::default()), (3, Hash::default())), + vec![1, 2, 4] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() ); // The set reachable from slot 1, excluding leaf 6, is just everything // except leaf 6 assert_eq!( - heaviest_subtree_fork_choice.subtree_diff(0, 6), - vec![0, 1, 3, 5, 2, 4].into_iter().collect::>() + heaviest_subtree_fork_choice.subtree_diff((0, Hash::default()), (6, Hash::default())), + vec![0, 1, 3, 5, 2, 4] + .into_iter() + .map(|s| (s, Hash::default())) + .collect::>() ); // Set root at 1 - heaviest_subtree_fork_choice.set_root(1); + heaviest_subtree_fork_choice.set_root((1, Hash::default())); // Zero no longer exists, set reachable from 0 is empty - assert!(heaviest_subtree_fork_choice.subtree_diff(0, 6).is_empty()); + assert!(heaviest_subtree_fork_choice + .subtree_diff((0, Hash::default()), (6, Hash::default())) + .is_empty()); } #[test] @@ -1648,7 +2691,7 @@ mod test { assert_eq!(tower.is_stray_last_vote(), false); assert_eq!( heaviest_subtree_fork_choice.heaviest_slot_on_same_voted_fork(&tower), - Some(2) + Some((2, Hash::default())) ); // Make slot 1 (existing in bank_forks) a restored stray slot @@ -1663,7 +2706,7 @@ mod test { assert_eq!(tower.is_stray_last_vote(), true); assert_eq!( heaviest_subtree_fork_choice.heaviest_slot_on_same_voted_fork(&tower), - Some(2) + Some((2, Hash::default())) ); // Make slot 3 (NOT existing in bank_forks) a restored stray slot @@ -1685,72 +2728,162 @@ mod test { let stake = 100; let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, stake); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![ - (vote_pubkeys[0], 6), - (vote_pubkeys[1], 6), - (vote_pubkeys[2], 2), + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], (6, Hash::default())), + (vote_pubkeys[1], (6, Hash::default())), + (vote_pubkeys[2], (2, Hash::default())), ]; let expected_best_slot = 6; assert_eq!( heaviest_subtree_fork_choice.add_votes( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule() ), - expected_best_slot, + (expected_best_slot, Hash::default()), ); // Mark slot 5 as invalid, the best fork should be its ancestor 3, // not the other for at 4. - let invalid_candidate = 5; - heaviest_subtree_fork_choice.mark_fork_invalid_candidate(invalid_candidate); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 3); + let invalid_candidate = (5, Hash::default()); + heaviest_subtree_fork_choice.mark_fork_invalid_candidate(&invalid_candidate); + assert_eq!(heaviest_subtree_fork_choice.best_overall_slot().0, 3); assert!(!heaviest_subtree_fork_choice - .is_candidate_slot(invalid_candidate) + .is_candidate_slot(&invalid_candidate) .unwrap()); // The ancestor is still a candidate - assert!(heaviest_subtree_fork_choice.is_candidate_slot(3).unwrap()); + assert!(heaviest_subtree_fork_choice + .is_candidate_slot(&(3, Hash::default())) + .unwrap()); // Adding another descendant to the invalid candidate won't // update the best slot, even if it contains votes let new_leaf_slot7 = 7; - heaviest_subtree_fork_choice.add_new_leaf_slot(new_leaf_slot7, Some(6)); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 3); - let pubkey_votes: Vec<(Pubkey, Slot)> = vec![(vote_pubkeys[0], new_leaf_slot7)]; + heaviest_subtree_fork_choice.add_new_leaf_slot( + (new_leaf_slot7, Hash::default()), + Some((6, Hash::default())), + ); let invalid_slot_ancestor = 3; + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot().0, + invalid_slot_ancestor + ); + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = + vec![(vote_pubkeys[0], (new_leaf_slot7, Hash::default()))]; assert_eq!( heaviest_subtree_fork_choice.add_votes( - &pubkey_votes, + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule() ), - invalid_slot_ancestor, + (invalid_slot_ancestor, Hash::default()), ); // Adding a descendant to the ancestor of the invalid candidate *should* update // the best slot though, since the ancestor is on the heaviest fork let new_leaf_slot8 = 8; - heaviest_subtree_fork_choice.add_new_leaf_slot(new_leaf_slot8, Some(invalid_slot_ancestor)); + heaviest_subtree_fork_choice.add_new_leaf_slot( + (new_leaf_slot8, Hash::default()), + Some((invalid_slot_ancestor, Hash::default())), + ); assert_eq!( - heaviest_subtree_fork_choice.best_overall_slot(), - new_leaf_slot8 + heaviest_subtree_fork_choice.best_overall_slot().0, + new_leaf_slot8, ); // If we mark slot a descendant of `invalid_candidate` as valid, then that // should also mark `invalid_candidate` as valid, and the best slot should // be the leaf of the heaviest fork, `new_leaf_slot`. - heaviest_subtree_fork_choice.mark_fork_valid_candidate(invalid_candidate); + heaviest_subtree_fork_choice.mark_fork_valid_candidate(&invalid_candidate); assert!(heaviest_subtree_fork_choice - .is_candidate_slot(invalid_candidate) + .is_candidate_slot(&invalid_candidate) .unwrap()); assert_eq!( - heaviest_subtree_fork_choice.best_overall_slot(), + heaviest_subtree_fork_choice.best_overall_slot().0, // Should pick the smaller slot of the two new equally weighted leaves new_leaf_slot7 ); } + #[test] + fn test_mark_valid_invalid_forks_duplicate() { + let ( + mut heaviest_subtree_fork_choice, + duplicate_leaves_descended_from_4, + duplicate_leaves_descended_from_5, + ) = setup_duplicate_forks(); + let stake = 100; + let (bank, vote_pubkeys) = bank_utils::setup_bank_and_vote_pubkeys(3, stake); + + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], duplicate_leaves_descended_from_4[0]), + (vote_pubkeys[1], duplicate_leaves_descended_from_5[0]), + ]; + + // The best slot should be the the smallest leaf descended from 4 + assert_eq!( + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule(), + ), + duplicate_leaves_descended_from_4[0] + ); + + // If we mark slot 4 as invalid, the ancestor 2 should be the heaviest, not + // the other branch at slot 5 + let invalid_candidate = (4, Hash::default()); + heaviest_subtree_fork_choice.mark_fork_invalid_candidate(&invalid_candidate); + + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + (2, Hash::default()) + ); + + // Marking candidate as valid again will choose the the heaviest leaf of + // the newly valid branch + let duplicate_slot = duplicate_leaves_descended_from_4[0].0; + let duplicate_descendant = (duplicate_slot + 1, Hash::new_unique()); + heaviest_subtree_fork_choice.add_new_leaf_slot( + duplicate_descendant, + Some(duplicate_leaves_descended_from_4[0]), + ); + heaviest_subtree_fork_choice.mark_fork_valid_candidate(&invalid_candidate); + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + duplicate_descendant + ); + + // Mark the current heaviest branch as invalid again + heaviest_subtree_fork_choice.mark_fork_invalid_candidate(&invalid_candidate); + + // If we add a new version of the duplicate slot that is not descended from the invalid + // candidate and votes for that duplicate slot, the new duplicate slot should be picked + // once it has more weight + let new_duplicate_hash = Hash::default(); + // The hash has to be smaller in order for the votes to be counted + assert!(new_duplicate_hash < duplicate_leaves_descended_from_4[0].1); + let new_duplicate = (duplicate_slot, new_duplicate_hash); + heaviest_subtree_fork_choice.add_new_leaf_slot(new_duplicate, Some((3, Hash::default()))); + + let pubkey_votes: Vec<(Pubkey, SlotHashKey)> = vec![ + (vote_pubkeys[0], new_duplicate), + (vote_pubkeys[1], new_duplicate), + ]; + + heaviest_subtree_fork_choice.add_votes( + pubkey_votes.iter(), + bank.epoch_stakes_map(), + bank.epoch_schedule(), + ); + + assert_eq!( + heaviest_subtree_fork_choice.best_overall_slot(), + new_duplicate + ); + } + fn setup_forks() -> HeaviestSubtreeForkChoice { /* Build fork structure: @@ -1769,9 +2902,75 @@ mod test { HeaviestSubtreeForkChoice::new_from_tree(forks) } + fn setup_duplicate_forks() -> ( + HeaviestSubtreeForkChoice, + Vec, + Vec, + ) { + /* + Build fork structure: + slot 0 + | + slot 1 + / \ + slot 2 | + | slot 3 + slot 4 \ + / \ slot 5 + slot 10 slot 10 / | \ + slot 6 slot 10 slot 10 + */ + + let mut heaviest_subtree_fork_choice = setup_forks(); + let duplicate_slot = 10; + let mut duplicate_leaves_descended_from_4 = + std::iter::repeat_with(|| (duplicate_slot, Hash::new_unique())) + .take(2) + .collect::>(); + let mut duplicate_leaves_descended_from_5 = + std::iter::repeat_with(|| (duplicate_slot, Hash::new_unique())) + .take(2) + .collect::>(); + duplicate_leaves_descended_from_4.sort(); + duplicate_leaves_descended_from_5.sort(); + + // Add versions of leaf 10, some with different ancestors, some with the same + // ancestors + for duplicate_leaf in &duplicate_leaves_descended_from_4 { + heaviest_subtree_fork_choice + .add_new_leaf_slot(*duplicate_leaf, Some((4, Hash::default()))); + } + for duplicate_leaf in &duplicate_leaves_descended_from_5 { + heaviest_subtree_fork_choice + .add_new_leaf_slot(*duplicate_leaf, Some((5, Hash::default()))); + } + + let mut dup_children = heaviest_subtree_fork_choice + .children(&(4, Hash::default())) + .unwrap() + .to_vec(); + dup_children.sort(); + assert_eq!(dup_children, duplicate_leaves_descended_from_4); + let mut dup_children: Vec<_> = heaviest_subtree_fork_choice + .children(&(5, Hash::default())) + .unwrap() + .iter() + .copied() + .filter(|(slot, _)| *slot == duplicate_slot) + .collect(); + dup_children.sort(); + assert_eq!(dup_children, duplicate_leaves_descended_from_5); + + ( + heaviest_subtree_fork_choice, + duplicate_leaves_descended_from_4, + duplicate_leaves_descended_from_5, + ) + } + fn check_process_update_correctness( heaviest_subtree_fork_choice: &mut HeaviestSubtreeForkChoice, - pubkey_votes: &[(Pubkey, Slot)], + pubkey_votes: &[(Pubkey, SlotHashKey)], slots_range: Range, bank: &Bank, stake: u64, @@ -1779,22 +2978,24 @@ mod test { ) where F: FnMut(Slot, &HeaviestSubtreeForkChoice) -> Slot, { - let unique_votes: HashSet = pubkey_votes - .iter() - .map(|(_, vote_slot)| *vote_slot) - .collect(); - let vote_ancestors: HashMap> = unique_votes + let unique_votes: HashSet = pubkey_votes.iter().map(|(_, (slot, _))| *slot).collect(); + let vote_ancestors: HashMap> = unique_votes .iter() .map(|v| { ( *v, - heaviest_subtree_fork_choice.ancestor_iterator(*v).collect(), + heaviest_subtree_fork_choice + .ancestor_iterator((*v, Hash::default())) + .collect(), ) }) .collect(); let mut vote_count: HashMap = HashMap::new(); for (_, vote) in pubkey_votes { - vote_count.entry(*vote).and_modify(|c| *c += 1).or_insert(1); + vote_count + .entry(vote.0) + .and_modify(|c| *c += 1) + .or_insert(1); } // Maps a slot to the number of descendants of that slot @@ -1805,7 +3006,8 @@ mod test { let num_voted_descendants = vote_ancestors .iter() .map(|(vote_slot, ancestors)| { - (ancestors.contains(&slot) || *vote_slot == slot) as usize + (ancestors.contains(&(slot, Hash::default())) || *vote_slot == slot) + as usize * vote_count.get(vote_slot).unwrap() }) .sum(); @@ -1813,13 +3015,13 @@ mod test { }) .collect(); - let update_operations = heaviest_subtree_fork_choice.generate_update_operations( - &pubkey_votes, + let update_operations_batch = heaviest_subtree_fork_choice.generate_update_operations( + pubkey_votes.iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); - heaviest_subtree_fork_choice.process_update_operations(update_operations); + heaviest_subtree_fork_choice.process_update_operations(update_operations_batch); for slot in slots_range { let expected_stake_voted_at = vote_count.get(&slot).cloned().unwrap_or(0) as u64 * stake; @@ -1827,17 +3029,22 @@ mod test { *num_voted_descendants.get(&slot).unwrap() as u64 * stake; assert_eq!( expected_stake_voted_at, - heaviest_subtree_fork_choice.stake_voted_at(slot).unwrap() + heaviest_subtree_fork_choice + .stake_voted_at(&(slot, Hash::default())) + .unwrap() ); assert_eq!( expected_stake_voted_subtree, heaviest_subtree_fork_choice - .stake_voted_subtree(slot) + .stake_voted_subtree(&(slot, Hash::default())) .unwrap() ); assert_eq!( expected_best_slot(slot, heaviest_subtree_fork_choice), - heaviest_subtree_fork_choice.best_slot(slot).unwrap() + heaviest_subtree_fork_choice + .best_slot(&(slot, Hash::default())) + .unwrap() + .0 ); } } diff --git a/core/src/progress_map.rs b/core/src/progress_map.rs index a6f2671ddd..98c1aa3e13 100644 --- a/core/src/progress_map.rs +++ b/core/src/progress_map.rs @@ -257,6 +257,7 @@ pub(crate) struct ForkStats { pub(crate) is_supermajority_confirmed: bool, pub(crate) computed: bool, pub(crate) lockout_intervals: LockoutIntervals, + pub(crate) bank_hash: Option, } #[derive(Clone, Default)] @@ -398,6 +399,12 @@ impl ProgressMap { .map(|fork_progress| fork_progress.is_dead) } + pub fn get_hash(&self, slot: Slot) -> Option { + self.progress_map + .get(&slot) + .and_then(|fork_progress| fork_progress.fork_stats.bank_hash) + } + pub fn is_propagated(&self, slot: Slot) -> bool { let leader_slot_to_check = self.get_latest_leader_slot(slot); diff --git a/core/src/repair_weight.rs b/core/src/repair_weight.rs index 9ebe46f3c5..26cce442e1 100644 --- a/core/src/repair_weight.rs +++ b/core/src/repair_weight.rs @@ -8,6 +8,7 @@ use solana_runtime::{contains::Contains, epoch_stakes::EpochStakes}; use solana_sdk::{ clock::Slot, epoch_schedule::{Epoch, EpochSchedule}, + hash::Hash, pubkey::Pubkey, }; use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; @@ -30,7 +31,7 @@ pub struct RepairWeight { impl RepairWeight { pub fn new(root: Slot) -> Self { - let root_tree = HeaviestSubtreeForkChoice::new(root); + let root_tree = HeaviestSubtreeForkChoice::new((root, Hash::default())); let slot_to_tree: HashMap = vec![(root, root)].into_iter().collect(); let trees: HashMap = vec![(root, root_tree)].into_iter().collect(); @@ -102,7 +103,12 @@ impl RepairWeight { new_ancestors.push_back(slot); if new_ancestors.len() > 1 { for i in 0..new_ancestors.len() - 1 { - tree.add_new_leaf_slot(new_ancestors[i + 1], Some(new_ancestors[i])); + // TODO: Repair right now does not distinguish between votes for different + // versions of the same slot. + tree.add_new_leaf_slot( + (new_ancestors[i + 1], Hash::default()), + Some((new_ancestors[i], Hash::default())), + ); self.slot_to_tree.insert(new_ancestors[i + 1], tree_root); } } @@ -122,7 +128,13 @@ impl RepairWeight { .get_mut(&tree_root) .expect("`slot_to_tree` and `self.trees` must be in sync"); let updates: Vec<_> = updates.into_iter().collect(); - tree.add_votes(&updates, epoch_stakes, epoch_schedule); + tree.add_votes( + updates + .iter() + .map(|(pubkey, slot)| (*pubkey, (*slot, Hash::default()))), + epoch_stakes, + epoch_schedule, + ); } } @@ -189,7 +201,9 @@ impl RepairWeight { .remove(&subtree_root) .expect("Must exist, was found in `self.trees` above"); self.remove_tree_slots( - subtree.all_slots_stake_voted_subtree().iter().map(|x| &x.0), + subtree + .all_slots_stake_voted_subtree() + .map(|((slot, _), _)| slot), new_root, ); } @@ -202,10 +216,16 @@ impl RepairWeight { // Find all descendants of `self.root` that are not reachable from `new_root`. // These are exactly the unrooted slots, which can be purged and added to // `self.unrooted_slots`. - let unrooted_slots = new_root_tree.subtree_diff(new_root_tree_root, new_root); - self.remove_tree_slots(unrooted_slots.iter(), new_root); + let unrooted_slots = new_root_tree.subtree_diff( + (new_root_tree_root, Hash::default()), + (new_root, Hash::default()), + ); + self.remove_tree_slots( + unrooted_slots.iter().map(|slot_hash| &slot_hash.0), + new_root, + ); - new_root_tree.set_root(new_root); + new_root_tree.set_root((new_root, Hash::default())); // Update `self.slot_to_tree` to reflect new root self.rename_tree_root(&new_root_tree, new_root); @@ -259,7 +279,7 @@ impl RepairWeight { .map(|(slot, tree)| { ( *slot, - tree.stake_voted_subtree(*slot) + tree.stake_voted_subtree(&(*slot, Hash::default())) .expect("Tree must have weight at its own root"), ) }) @@ -343,7 +363,7 @@ impl RepairWeight { for ancestor in new_ancestors.iter().skip(num_skip).rev() { self.slot_to_tree.insert(*ancestor, orphan_tree_root); - heaviest_tree.add_root_parent(*ancestor); + heaviest_tree.add_root_parent((*ancestor, Hash::default())); } } if let Some(parent_tree_root) = parent_tree_root { @@ -386,8 +406,7 @@ impl RepairWeight { self.remove_tree_slots( orphan_tree .all_slots_stake_voted_subtree() - .iter() - .map(|x| &x.0), + .map(|((slot, _), _)| slot), self.root, ); None @@ -403,8 +422,10 @@ impl RepairWeight { // Update `self.slot_to_tree` self.slot_to_tree.insert(new_tree_root, new_tree_root); - self.trees - .insert(new_tree_root, HeaviestSubtreeForkChoice::new(new_tree_root)); + self.trees.insert( + new_tree_root, + HeaviestSubtreeForkChoice::new((new_tree_root, Hash::default())), + ); } fn find_ancestor_subtree_of_slot( @@ -460,13 +481,18 @@ impl RepairWeight { .get_mut(&root2) .expect("tree to be merged into must exist"); - tree2.merge(tree1, merge_leaf, epoch_stakes, epoch_schedule); + tree2.merge( + tree1, + &(merge_leaf, Hash::default()), + epoch_stakes, + epoch_schedule, + ); } // Update all slots in the `tree1` to point to `root2`, fn rename_tree_root(&mut self, tree1: &HeaviestSubtreeForkChoice, root2: Slot) { let all_slots = tree1.all_slots_stake_voted_subtree(); - for (slot, _) in all_slots { + for ((slot, _), _) in all_slots { *self .slot_to_tree .get_mut(&slot) @@ -560,7 +586,14 @@ mod test { // repair_weight should contain one subtree 0->1 assert_eq!(repair_weight.trees.len(), 1); - assert_eq!(repair_weight.trees.get(&0).unwrap().ancestors(1), vec![0]); + assert_eq!( + repair_weight + .trees + .get(&0) + .unwrap() + .ancestors((1, Hash::default())), + vec![(0, Hash::default())] + ); for i in &[0, 1] { assert_eq!(*repair_weight.slot_to_tree.get(i).unwrap(), 0); } @@ -577,11 +610,25 @@ mod test { ); assert_eq!(repair_weight.trees.len(), 1); assert_eq!( - repair_weight.trees.get(&0).unwrap().ancestors(4), + repair_weight + .trees + .get(&0) + .unwrap() + .ancestors((4, Hash::default())) + .into_iter() + .map(|slot_hash| slot_hash.0) + .collect::>(), vec![2, 1, 0] ); assert_eq!( - repair_weight.trees.get(&0).unwrap().ancestors(6), + repair_weight + .trees + .get(&0) + .unwrap() + .ancestors((6, Hash::default())) + .into_iter() + .map(|slot_hash| slot_hash.0) + .collect::>(), vec![5, 3, 1, 0] ); for slot in 0..=6 { @@ -590,7 +637,7 @@ mod test { .trees .get(&0) .unwrap() - .stake_voted_at(slot) + .stake_voted_at(&(slot, Hash::default())) .unwrap(); if slot == 6 { assert_eq!(stake_voted_at, 3 * stake); @@ -604,7 +651,7 @@ mod test { .trees .get(&0) .unwrap() - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(); assert_eq!(stake_voted_subtree, 3 * stake); } @@ -613,7 +660,7 @@ mod test { .trees .get(&0) .unwrap() - .stake_voted_subtree(*slot) + .stake_voted_subtree(&(*slot, Hash::default())) .unwrap(); assert_eq!(stake_voted_subtree, 0); } @@ -637,8 +684,20 @@ mod test { // Should contain two trees, one for main fork, one for the orphan // branch assert_eq!(repair_weight.trees.len(), 2); - assert_eq!(repair_weight.trees.get(&0).unwrap().ancestors(1), vec![0]); - assert!(repair_weight.trees.get(&8).unwrap().ancestors(8).is_empty()); + assert_eq!( + repair_weight + .trees + .get(&0) + .unwrap() + .ancestors((1, Hash::default())), + vec![(0, Hash::default())] + ); + assert!(repair_weight + .trees + .get(&8) + .unwrap() + .ancestors((8, Hash::default())) + .is_empty()); let votes = vec![(1, vote_pubkeys.clone()), (10, vote_pubkeys.clone())]; let mut repair_weight = RepairWeight::new(0); @@ -652,8 +711,22 @@ mod test { // Should contain two trees, one for main fork, one for the orphan // branch assert_eq!(repair_weight.trees.len(), 2); - assert_eq!(repair_weight.trees.get(&0).unwrap().ancestors(1), vec![0]); - assert_eq!(repair_weight.trees.get(&8).unwrap().ancestors(10), vec![8]); + assert_eq!( + repair_weight + .trees + .get(&0) + .unwrap() + .ancestors((1, Hash::default())), + vec![(0, Hash::default())] + ); + assert_eq!( + repair_weight + .trees + .get(&8) + .unwrap() + .ancestors((10, Hash::default())), + vec![(8, Hash::default())] + ); // Connect orphan back to main fork blockstore.add_tree(tr(6) / (tr(8)), true, true, 2, Hash::default()); @@ -672,7 +745,14 @@ mod test { bank.epoch_schedule(), ); assert_eq!( - repair_weight.trees.get(&8).unwrap().ancestors(11), + repair_weight + .trees + .get(&8) + .unwrap() + .ancestors((11, Hash::default())) + .into_iter() + .map(|slot_hash| slot_hash.0) + .collect::>(), vec![10, 8] ); @@ -784,13 +864,13 @@ mod test { .trees .get(&8) .unwrap() - .stake_voted_subtree(8) + .stake_voted_subtree(&(8, Hash::default())) .unwrap(), repair_weight .trees .get(&20) .unwrap() - .stake_voted_subtree(20) + .stake_voted_subtree(&(20, Hash::default())) .unwrap() ); assert_eq!(repairs.len(), 1); @@ -929,11 +1009,11 @@ mod test { assert_eq!(*repair_weight.slot_to_tree.get(&2).unwrap(), 1); // Trees tracked should be updated - assert_eq!(repair_weight.trees.get(&1).unwrap().root(), 1); + assert_eq!(repair_weight.trees.get(&1).unwrap().root().0, 1); // Orphan slots should not be changed for orphan in &[8, 20] { - assert_eq!(repair_weight.trees.get(orphan).unwrap().root(), *orphan); + assert_eq!(repair_weight.trees.get(orphan).unwrap().root().0, *orphan); assert_eq!(repair_weight.slot_to_tree.get(orphan).unwrap(), orphan); } } @@ -957,7 +1037,7 @@ mod test { // Orphan slots should not be changed for orphan in &[8, 20] { - assert_eq!(repair_weight.trees.get(orphan).unwrap().root(), *orphan); + assert_eq!(repair_weight.trees.get(orphan).unwrap().root().0, *orphan); assert_eq!(repair_weight.slot_to_tree.get(orphan).unwrap(), orphan); } } @@ -979,7 +1059,7 @@ mod test { assert!(!repair_weight.slot_to_tree.contains_key(&8)); // Other higher orphan branch rooted at slot `20` remains unchanged - assert_eq!(repair_weight.trees.get(&20).unwrap().root(), 20); + assert_eq!(repair_weight.trees.get(&20).unwrap().root().0, 20); assert_eq!(*repair_weight.slot_to_tree.get(&20).unwrap(), 20); } @@ -1020,7 +1100,7 @@ mod test { // Orphan 20 should still exist assert_eq!(repair_weight.trees.len(), 2); - assert_eq!(repair_weight.trees.get(&20).unwrap().root(), 20); + assert_eq!(repair_weight.trees.get(&20).unwrap().root().0, 20); assert_eq!(*repair_weight.slot_to_tree.get(&20).unwrap(), 20); // Now set root at a slot 30 that doesnt exist in `repair_weight`, but is @@ -1191,7 +1271,7 @@ mod test { // Check orphans are present for orphan in &[8, 20] { - assert_eq!(repair_weight.trees.get(orphan).unwrap().root(), *orphan); + assert_eq!(repair_weight.trees.get(orphan).unwrap().root().0, *orphan); assert_eq!(repair_weight.slot_to_tree.get(orphan).unwrap(), orphan); } (blockstore, bank, repair_weight) @@ -1208,7 +1288,10 @@ mod test { assert!(!repair_weight.unrooted_slots.contains(&old_root)); // Validate new root - assert_eq!(repair_weight.trees.get(&new_root).unwrap().root(), new_root); + assert_eq!( + repair_weight.trees.get(&new_root).unwrap().root().0, + new_root + ); assert_eq!( *repair_weight.slot_to_tree.get(&new_root).unwrap(), new_root diff --git a/core/src/repair_weighted_traversal.rs b/core/src/repair_weighted_traversal.rs index 59371a7c3d..534ef4841d 100644 --- a/core/src/repair_weighted_traversal.rs +++ b/core/src/repair_weighted_traversal.rs @@ -4,7 +4,7 @@ use crate::{ }; use solana_ledger::blockstore::Blockstore; use solana_runtime::contains::Contains; -use solana_sdk::clock::Slot; +use solana_sdk::{clock::Slot, hash::Hash}; use std::collections::{HashMap, HashSet}; #[derive(Debug, PartialEq)] @@ -32,7 +32,7 @@ impl<'a> RepairWeightTraversal<'a> { pub fn new(tree: &'a HeaviestSubtreeForkChoice) -> Self { Self { tree, - pending: vec![Visit::Unvisited(tree.root())], + pending: vec![Visit::Unvisited(tree.root().0)], } } } @@ -48,16 +48,20 @@ impl<'a> Iterator for RepairWeightTraversal<'a> { self.pending.push(Visit::Visited(slot)); let mut children: Vec<_> = self .tree - .children(slot) + .children(&(slot, Hash::default())) .unwrap() .iter() - .map(|child_slot| Visit::Unvisited(*child_slot)) + .map(|(child_slot, _)| Visit::Unvisited(*child_slot)) .collect(); // Sort children by weight to prioritize visiting the heaviest // ones first - children - .sort_by(|slot1, slot2| self.tree.max_by_weight(slot1.slot(), slot2.slot())); + children.sort_by(|slot1, slot2| { + self.tree.max_by_weight( + (slot1.slot(), Hash::default()), + (slot2.slot(), Hash::default()), + ) + }); self.pending.extend(children); } next @@ -87,6 +91,9 @@ pub fn get_best_repair_shreds<'a>( .entry(next.slot()) .or_insert_with(|| blockstore.meta(next.slot()).unwrap()); + // May not exist if blockstore purged the SlotMeta due to something + // like duplicate slots. TODO: Account for duplicate slot may be in orphans, especially + // if earlier duplicate was already removed if let Some(slot_meta) = slot_meta { match next { Visit::Unvisited(slot) => { @@ -137,7 +144,7 @@ pub mod test { #[test] fn test_weighted_repair_traversal_single() { - let heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new(42); + let heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new((42, Hash::default())); let weighted_traversal = RepairWeightTraversal::new(&heaviest_subtree_fork_choice); let steps: Vec<_> = weighted_traversal.collect(); assert_eq!(steps, vec![Visit::Unvisited(42), Visit::Visited(42)]); @@ -174,7 +181,7 @@ pub mod test { // Add a vote to branch with slot 5, // should prioritize that branch heaviest_subtree_fork_choice.add_votes( - &[(vote_pubkeys[0], 5)], + [(vote_pubkeys[0], (5, Hash::default()))].iter(), bank.epoch_stakes_map(), bank.epoch_schedule(), ); @@ -227,8 +234,8 @@ pub mod test { // Add some leaves to blockstore, attached to the current best leaf, should prioritize // repairing those new leaves before trying other branches repairs = vec![]; - let best_overall_slot = heaviest_subtree_fork_choice.best_overall_slot(); - assert_eq!(heaviest_subtree_fork_choice.best_overall_slot(), 4); + let best_overall_slot = heaviest_subtree_fork_choice.best_overall_slot().0; + assert_eq!(best_overall_slot, 4); blockstore.add_tree( tr(best_overall_slot) / (tr(6) / tr(7)), true, @@ -406,6 +413,7 @@ pub mod test { slot 4 | slot 5 */ + let forks = tr(0) / (tr(1) / (tr(2) / (tr(4))) / (tr(3) / (tr(5)))); let ledger_path = get_tmp_ledger_path!(); let blockstore = Blockstore::open(&ledger_path).unwrap(); diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index c3899e0d71..919d183fd1 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -753,8 +753,10 @@ impl ReplayStage { ); } let root = root_bank.slot(); - let heaviest_subtree_fork_choice = - HeaviestSubtreeForkChoice::new_from_frozen_banks(root, &frozen_banks); + let heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new_from_frozen_banks( + (root, root_bank.hash()), + &frozen_banks, + ); (progress, heaviest_subtree_fork_choice) } @@ -1609,8 +1611,12 @@ impl ReplayStage { transaction_status_sender.send_transaction_status_freeze_message(&bank); } bank.freeze(); - heaviest_subtree_fork_choice - .add_new_leaf_slot(bank.slot(), Some(bank.parent_slot())); + let bank_hash = bank.hash(); + assert_ne!(bank_hash, Hash::default()); + heaviest_subtree_fork_choice.add_new_leaf_slot( + (bank.slot(), bank.hash()), + Some((bank.parent_slot(), bank.parent_hash())), + ); check_slot_agrees_with_cluster( bank.slot(), bank_forks.read().unwrap().root(), @@ -1652,7 +1658,7 @@ impl ReplayStage { vote_tracker: &VoteTracker, cluster_slots: &ClusterSlots, bank_forks: &RwLock, - heaviest_subtree_fork_choice: &mut dyn ForkChoice, + heaviest_subtree_fork_choice: &mut HeaviestSubtreeForkChoice, ) -> Vec { frozen_banks.sort_by_key(|bank| bank.slot()); let mut new_stats = vec![]; @@ -1694,6 +1700,7 @@ impl ReplayStage { stats.voted_stakes = voted_stakes; stats.lockout_intervals = lockout_intervals; stats.block_height = bank.block_height(); + stats.bank_hash = Some(bank.hash()); stats.computed = true; new_stats.push(bank_slot); datapoint_info!( @@ -2202,9 +2209,9 @@ impl ReplayStage { } } progress.handle_new_root(&r_bank_forks); - heaviest_subtree_fork_choice.set_root(new_root); + heaviest_subtree_fork_choice.set_root((new_root, r_bank_forks.root_bank().hash())); let mut slots_ge_root = gossip_duplicate_confirmed_slots.split_off(&new_root); - // gossip_duplicate_confirmed_slots now only contains entries >= `new_root` + // gossip_confirmed_slots now only contains entries >= `new_root` std::mem::swap(gossip_duplicate_confirmed_slots, &mut slots_ge_root); let mut slots_ge_root = gossip_verified_vote_hashes.split_off(&new_root); @@ -2598,14 +2605,19 @@ pub(crate) mod tests { let genesis_config = create_genesis_config(10_000).genesis_config; let bank0 = Bank::new(&genesis_config); let bank_forks = Arc::new(RwLock::new(BankForks::new(bank0))); + let root = 3; - let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new(root); let root_bank = Bank::new_from_parent( bank_forks.read().unwrap().get(0).unwrap(), &Pubkey::default(), root, ); + root_bank.freeze(); + let root_hash = root_bank.hash(); bank_forks.write().unwrap().insert(root_bank); + + let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new((root, root_hash)); + let mut progress = ProgressMap::default(); for i in 0..=root { progress.insert( @@ -2680,8 +2692,10 @@ pub(crate) mod tests { &Pubkey::default(), root, ); + root_bank.freeze(); + let root_hash = root_bank.hash(); bank_forks.write().unwrap().insert(root_bank); - let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new(root); + let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new((root, root_hash)); let mut progress = ProgressMap::default(); for i in 0..=root { progress.insert( @@ -2975,7 +2989,7 @@ pub(crate) mod tests { &HashMap::new(), &HashMap::new(), &mut progress, - &mut HeaviestSubtreeForkChoice::new(0), + &mut HeaviestSubtreeForkChoice::new((0, Hash::default())), ); } @@ -3376,8 +3390,7 @@ pub(crate) mod tests { // Create the tree of banks in a BankForks object let forks = tr(0) / (tr(1)) / (tr(2)); - vote_simulator.fill_bank_forks(forks.clone(), &HashMap::new()); - let mut heaviest_subtree_fork_choice = HeaviestSubtreeForkChoice::new_from_tree(forks); + vote_simulator.fill_bank_forks(forks, &HashMap::new()); let mut frozen_banks: Vec<_> = vote_simulator .bank_forks .read() @@ -3386,6 +3399,7 @@ pub(crate) mod tests { .values() .cloned() .collect(); + let mut heaviest_subtree_fork_choice = &mut vote_simulator.heaviest_subtree_fork_choice; let ancestors = vote_simulator.bank_forks.read().unwrap().ancestors(); ReplayStage::compute_bank_stats( @@ -3400,9 +3414,27 @@ pub(crate) mod tests { &mut heaviest_subtree_fork_choice, ); + let bank1 = vote_simulator + .bank_forks + .read() + .unwrap() + .get(1) + .unwrap() + .clone(); + let bank2 = vote_simulator + .bank_forks + .read() + .unwrap() + .get(2) + .unwrap() + .clone(); assert_eq!( - heaviest_subtree_fork_choice.stake_voted_subtree(1).unwrap(), - heaviest_subtree_fork_choice.stake_voted_subtree(2).unwrap() + heaviest_subtree_fork_choice + .stake_voted_subtree(&(1, bank1.hash())) + .unwrap(), + heaviest_subtree_fork_choice + .stake_voted_subtree(&(2, bank2.hash())) + .unwrap() ); let (heaviest_bank, _) = heaviest_subtree_fork_choice.select_forks( @@ -3480,8 +3512,9 @@ pub(crate) mod tests { assert_eq!( vote_simulator .heaviest_subtree_fork_choice - .best_slot(bank.slot()) - .unwrap(), + .best_slot(&(bank.slot(), bank.hash())) + .unwrap() + .0, 3 ); } diff --git a/core/src/tree_diff.rs b/core/src/tree_diff.rs index bad6d31437..358c49d4ed 100644 --- a/core/src/tree_diff.rs +++ b/core/src/tree_diff.rs @@ -1,30 +1,30 @@ -use solana_sdk::clock::Slot; -use std::collections::HashSet; +use std::{collections::HashSet, hash::Hash}; pub trait TreeDiff { - fn children(&self, slot: Slot) -> Option<&[Slot]>; + type TreeKey: Hash + PartialEq + Eq + Copy; + fn children(&self, key: &Self::TreeKey) -> Option<&[Self::TreeKey]>; - fn contains_slot(&self, slot: Slot) -> bool; + fn contains_slot(&self, slot: &Self::TreeKey) -> bool; // Find all nodes reachable from `root1`, excluding subtree at `root2` - fn subtree_diff(&self, root1: Slot, root2: Slot) -> HashSet { - if !self.contains_slot(root1) { + fn subtree_diff(&self, root1: Self::TreeKey, root2: Self::TreeKey) -> HashSet { + if !self.contains_slot(&root1) { return HashSet::new(); } - let mut pending_slots = vec![root1]; + let mut pending_keys = vec![root1]; let mut reachable_set = HashSet::new(); - while !pending_slots.is_empty() { - let current_slot = pending_slots.pop().unwrap(); - if current_slot == root2 { + while !pending_keys.is_empty() { + let current_key = pending_keys.pop().unwrap(); + if current_key == root2 { continue; } - reachable_set.insert(current_slot); for child in self - .children(current_slot) + .children(¤t_key) .expect("slot was discovered earlier, must exist") { - pending_slots.push(*child); + pending_keys.push(*child); } + reachable_set.insert(current_key); } reachable_set diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 32450d7179..ae64453028 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -59,6 +59,10 @@ impl Vote { pub fn last_voted_slot(&self) -> Option { self.slots.last().copied() } + + pub fn last_voted_slot_hash(&self) -> Option<(Slot, Hash)> { + self.slots.last().copied().map(|slot| (slot, self.hash)) + } } #[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone, AbiExample)] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5db227584a..a806d903da 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2127,6 +2127,10 @@ impl Bank { self.parent_slot } + pub fn parent_hash(&self) -> Hash { + self.parent_hash + } + fn process_genesis_config(&mut self, genesis_config: &GenesisConfig) { // Bootstrap validator collects fees until `new_from_parent` is called. self.fee_rate_governor = genesis_config.fee_rate_governor.clone(); diff --git a/runtime/src/bank_forks.rs b/runtime/src/bank_forks.rs index f01ebabd13..fda46e11c9 100644 --- a/runtime/src/bank_forks.rs +++ b/runtime/src/bank_forks.rs @@ -6,7 +6,7 @@ use crate::{ }; use log::*; use solana_metrics::inc_new_counter_info; -use solana_sdk::{clock::Slot, timing}; +use solana_sdk::{clock::Slot, hash::Hash, timing}; use std::{ collections::{hash_map::Entry, HashMap, HashSet}, ops::Index, @@ -106,6 +106,17 @@ impl BankForks { self.banks.get(&bank_slot) } + pub fn get_with_checked_hash( + &self, + (bank_slot, expected_hash): (Slot, Hash), + ) -> Option<&Arc> { + let maybe_bank = self.banks.get(&bank_slot); + if let Some(bank) = maybe_bank { + assert_eq!(bank.hash(), expected_hash); + } + maybe_bank + } + pub fn root_bank(&self) -> Arc { self[self.root()].clone() }