Port unconfirmed duplicate tracking logic from ProgressMap to ForkChoice (#17779)

This commit is contained in:
carllin
2021-06-11 03:09:57 -07:00
committed by GitHub
parent da9ae5f836
commit c8535be0e1
5 changed files with 930 additions and 689 deletions

View File

@@ -161,7 +161,6 @@ pub(crate) struct ForkProgress {
pub(crate) propagated_stats: PropagatedStats,
pub(crate) replay_stats: ReplaySlotStats,
pub(crate) replay_progress: ConfirmationProgress,
pub(crate) duplicate_stats: DuplicateStats,
// Note `num_blocks_on_fork` and `num_dropped_blocks_on_fork` only
// count new blocks replayed since last restart, which won't include
// blocks already existing in the ledger/before snapshot at start,
@@ -174,7 +173,6 @@ impl ForkProgress {
pub fn new(
last_entry: Hash,
prev_leader_slot: Option<Slot>,
duplicate_stats: DuplicateStats,
validator_stake_info: Option<ValidatorStakeInfo>,
num_blocks_on_fork: u64,
num_dropped_blocks_on_fork: u64,
@@ -208,7 +206,6 @@ impl ForkProgress {
fork_stats: ForkStats::default(),
replay_stats: ReplaySlotStats::default(),
replay_progress: ConfirmationProgress::new(last_entry),
duplicate_stats,
num_blocks_on_fork,
num_dropped_blocks_on_fork,
propagated_stats: PropagatedStats {
@@ -228,7 +225,6 @@ impl ForkProgress {
my_pubkey: &Pubkey,
voting_pubkey: &Pubkey,
prev_leader_slot: Option<Slot>,
duplicate_stats: DuplicateStats,
num_blocks_on_fork: u64,
num_dropped_blocks_on_fork: u64,
) -> Self {
@@ -247,20 +243,11 @@ impl ForkProgress {
Self::new(
bank.last_blockhash(),
prev_leader_slot,
duplicate_stats,
validator_stake_info,
num_blocks_on_fork,
num_dropped_blocks_on_fork,
)
}
pub fn is_duplicate_confirmed(&self) -> bool {
self.duplicate_stats.is_duplicate_confirmed
}
pub fn set_duplicate_confirmed(&mut self) {
self.duplicate_stats.set_duplicate_confirmed();
}
}
#[derive(Debug, Clone, Default)]
@@ -295,38 +282,6 @@ pub(crate) struct PropagatedStats {
pub(crate) total_epoch_stake: u64,
}
#[derive(Clone, Default)]
pub(crate) struct DuplicateStats {
latest_unconfirmed_duplicate_ancestor: Option<Slot>,
is_duplicate_confirmed: bool,
}
impl DuplicateStats {
pub fn new_with_unconfirmed_duplicate_ancestor(
latest_unconfirmed_duplicate_ancestor: Option<Slot>,
) -> Self {
Self {
latest_unconfirmed_duplicate_ancestor,
is_duplicate_confirmed: false,
}
}
fn set_duplicate_confirmed(&mut self) {
self.is_duplicate_confirmed = true;
self.latest_unconfirmed_duplicate_ancestor = None;
}
fn update_with_newly_confirmed_duplicate_ancestor(&mut self, newly_confirmed_ancestor: Slot) {
if let Some(latest_unconfirmed_duplicate_ancestor) =
self.latest_unconfirmed_duplicate_ancestor
{
if latest_unconfirmed_duplicate_ancestor <= newly_confirmed_ancestor {
self.latest_unconfirmed_duplicate_ancestor = None;
}
}
}
}
impl PropagatedStats {
pub fn add_vote_pubkey(&mut self, vote_pubkey: Pubkey, stake: u64) {
if self.propagated_validators.insert(vote_pubkey) {
@@ -458,102 +413,6 @@ impl ProgressMap {
}
}
#[cfg(test)]
pub fn is_unconfirmed_duplicate(&self, slot: Slot) -> Option<bool> {
self.get(&slot).map(|p| {
p.duplicate_stats
.latest_unconfirmed_duplicate_ancestor
.map(|ancestor| ancestor == slot)
.unwrap_or(false)
})
}
pub fn latest_unconfirmed_duplicate_ancestor(&self, slot: Slot) -> Option<Slot> {
self.get(&slot)
.map(|p| p.duplicate_stats.latest_unconfirmed_duplicate_ancestor)
.unwrap_or(None)
}
pub fn set_unconfirmed_duplicate_slot(&mut self, slot: Slot, descendants: &HashSet<u64>) {
if let Some(fork_progress) = self.get_mut(&slot) {
if fork_progress.is_duplicate_confirmed() {
assert!(fork_progress
.duplicate_stats
.latest_unconfirmed_duplicate_ancestor
.is_none());
return;
}
if fork_progress
.duplicate_stats
.latest_unconfirmed_duplicate_ancestor
== Some(slot)
{
// Already been marked
return;
}
fork_progress
.duplicate_stats
.latest_unconfirmed_duplicate_ancestor = Some(slot);
for d in descendants {
if let Some(fork_progress) = self.get_mut(&d) {
fork_progress
.duplicate_stats
.latest_unconfirmed_duplicate_ancestor = Some(std::cmp::max(
fork_progress
.duplicate_stats
.latest_unconfirmed_duplicate_ancestor
.unwrap_or(0),
slot,
));
}
}
}
}
pub fn set_confirmed_duplicate_slot(
&mut self,
slot: Slot,
ancestors: &HashSet<u64>,
descendants: &HashSet<u64>,
) {
for a in ancestors {
if let Some(fork_progress) = self.get_mut(&a) {
fork_progress.set_duplicate_confirmed();
}
}
if let Some(slot_fork_progress) = self.get_mut(&slot) {
// Setting the fields here is nly correct and necessary if the loop above didn't
// already do this, so check with an assert.
assert!(!ancestors.contains(&slot));
let slot_had_unconfirmed_duplicate_ancestor = slot_fork_progress
.duplicate_stats
.latest_unconfirmed_duplicate_ancestor
.is_some();
slot_fork_progress.set_duplicate_confirmed();
if slot_had_unconfirmed_duplicate_ancestor {
for d in descendants {
if let Some(descendant_fork_progress) = self.get_mut(&d) {
descendant_fork_progress
.duplicate_stats
.update_with_newly_confirmed_duplicate_ancestor(slot);
}
}
} else {
// Neither this slot `S`, nor earlier ancestors were marked as duplicate,
// so this means all descendants either:
// 1) Have no duplicate ancestors
// 2) Have a duplicate ancestor > `S`
// In both cases, there's no need to iterate through descendants because
// this confirmation on `S` is irrelevant to them.
}
}
}
pub fn my_latest_landed_vote(&self, slot: Slot) -> Option<Slot> {
self.progress_map
.get(&slot)
@@ -571,12 +430,6 @@ impl ProgressMap {
.map(|s| s.fork_stats.is_supermajority_confirmed)
}
pub fn is_duplicate_confirmed(&self, slot: Slot) -> Option<bool> {
self.progress_map
.get(&slot)
.map(|s| s.is_duplicate_confirmed())
}
pub fn get_bank_prev_leader_slot(&self, bank: &Bank) -> Option<Slot> {
let parent_slot = bank.parent_slot();
self.get_propagated_stats(parent_slot)
@@ -619,8 +472,6 @@ impl ProgressMap {
#[cfg(test)]
mod test {
use super::*;
use crate::consensus::test::VoteSimulator;
use trees::tr;
#[test]
fn test_add_vote_pubkey() {
@@ -711,21 +562,13 @@ mod test {
fn test_is_propagated_status_on_construction() {
// If the given ValidatorStakeInfo == None, then this is not
// a leader slot and is_propagated == false
let progress = ForkProgress::new(
Hash::default(),
Some(9),
DuplicateStats::default(),
None,
0,
0,
);
let progress = ForkProgress::new(Hash::default(), Some(9), None, 0, 0);
assert!(!progress.propagated_stats.is_propagated);
// If the stake is zero, then threshold is always achieved
let progress = ForkProgress::new(
Hash::default(),
Some(9),
DuplicateStats::default(),
Some(ValidatorStakeInfo {
total_epoch_stake: 0,
..ValidatorStakeInfo::default()
@@ -740,7 +583,6 @@ mod test {
let progress = ForkProgress::new(
Hash::default(),
Some(9),
DuplicateStats::default(),
Some(ValidatorStakeInfo {
total_epoch_stake: 2,
..ValidatorStakeInfo::default()
@@ -754,7 +596,6 @@ mod test {
let progress = ForkProgress::new(
Hash::default(),
Some(9),
DuplicateStats::default(),
Some(ValidatorStakeInfo {
stake: 1,
total_epoch_stake: 2,
@@ -771,7 +612,6 @@ mod test {
let progress = ForkProgress::new(
Hash::default(),
Some(9),
DuplicateStats::default(),
Some(ValidatorStakeInfo::default()),
0,
0,
@@ -785,23 +625,12 @@ mod test {
// Insert new ForkProgress for slot 10 (not a leader slot) and its
// previous leader slot 9 (leader slot)
progress_map.insert(
10,
ForkProgress::new(
Hash::default(),
Some(9),
DuplicateStats::default(),
None,
0,
0,
),
);
progress_map.insert(10, ForkProgress::new(Hash::default(), Some(9), None, 0, 0));
progress_map.insert(
9,
ForkProgress::new(
Hash::default(),
None,
DuplicateStats::default(),
Some(ValidatorStakeInfo::default()),
0,
0,
@@ -816,17 +645,7 @@ mod test {
// The previous leader before 8, slot 7, does not exist in
// progress map, so is_propagated(8) should return true as
// this implies the parent is rooted
progress_map.insert(
8,
ForkProgress::new(
Hash::default(),
Some(7),
DuplicateStats::default(),
None,
0,
0,
),
);
progress_map.insert(8, ForkProgress::new(Hash::default(), Some(7), None, 0, 0));
assert!(progress_map.is_propagated(8));
// If we set the is_propagated = true, is_propagated should return true
@@ -849,157 +668,4 @@ mod test {
.is_leader_slot = true;
assert!(!progress_map.is_propagated(10));
}
fn setup_set_unconfirmed_and_confirmed_duplicate_slot_tests(
smaller_duplicate_slot: Slot,
larger_duplicate_slot: Slot,
) -> (ProgressMap, RwLock<BankForks>) {
// Create simple fork 0 -> 1 -> 2 -> 3 -> 4 -> 5
let forks = tr(0) / (tr(1) / (tr(2) / (tr(3) / (tr(4) / tr(5)))));
let mut vote_simulator = VoteSimulator::new(1);
vote_simulator.fill_bank_forks(forks, &HashMap::new());
let VoteSimulator {
mut progress,
bank_forks,
..
} = vote_simulator;
let descendants = bank_forks.read().unwrap().descendants().clone();
// Mark the slots as unconfirmed duplicates
progress.set_unconfirmed_duplicate_slot(
smaller_duplicate_slot,
&descendants.get(&smaller_duplicate_slot).unwrap(),
);
progress.set_unconfirmed_duplicate_slot(
larger_duplicate_slot,
&descendants.get(&larger_duplicate_slot).unwrap(),
);
// Correctness checks
for slot in bank_forks.read().unwrap().banks().keys() {
if *slot < smaller_duplicate_slot {
assert!(progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.is_none());
} else if *slot < larger_duplicate_slot {
assert_eq!(
progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.unwrap(),
smaller_duplicate_slot
);
} else {
assert_eq!(
progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.unwrap(),
larger_duplicate_slot
);
}
}
(progress, bank_forks)
}
#[test]
fn test_set_unconfirmed_duplicate_confirm_smaller_slot_first() {
let smaller_duplicate_slot = 1;
let larger_duplicate_slot = 4;
let (mut progress, bank_forks) = setup_set_unconfirmed_and_confirmed_duplicate_slot_tests(
smaller_duplicate_slot,
larger_duplicate_slot,
);
let descendants = bank_forks.read().unwrap().descendants().clone();
let ancestors = bank_forks.read().unwrap().ancestors();
// Mark the smaller duplicate slot as confirmed
progress.set_confirmed_duplicate_slot(
smaller_duplicate_slot,
&ancestors.get(&smaller_duplicate_slot).unwrap(),
&descendants.get(&smaller_duplicate_slot).unwrap(),
);
for slot in bank_forks.read().unwrap().banks().keys() {
if *slot < larger_duplicate_slot {
// Only slots <= smaller_duplicate_slot have been duplicate confirmed
if *slot <= smaller_duplicate_slot {
assert!(progress.is_duplicate_confirmed(*slot).unwrap());
} else {
assert!(!progress.is_duplicate_confirmed(*slot).unwrap());
}
// The unconfirmed duplicate flag has been cleared on the smaller
// descendants because their most recent duplicate ancestor has
// been confirmed
assert!(progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.is_none());
} else {
assert!(!progress.is_duplicate_confirmed(*slot).unwrap(),);
// The unconfirmed duplicate flag has not been cleared on the smaller
// descendants because their most recent duplicate ancestor,
// `larger_duplicate_slot` has not yet been confirmed
assert_eq!(
progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.unwrap(),
larger_duplicate_slot
);
}
}
// Mark the larger duplicate slot as confirmed, all slots should no longer
// have any unconfirmed duplicate ancestors, and should be marked as duplciate confirmed
progress.set_confirmed_duplicate_slot(
larger_duplicate_slot,
&ancestors.get(&larger_duplicate_slot).unwrap(),
&descendants.get(&larger_duplicate_slot).unwrap(),
);
for slot in bank_forks.read().unwrap().banks().keys() {
// All slots <= the latest duplciate confirmed slot are ancestors of
// that slot, so they should all be marked duplicate confirmed
assert_eq!(
progress.is_duplicate_confirmed(*slot).unwrap(),
*slot <= larger_duplicate_slot
);
assert!(progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.is_none());
}
}
#[test]
fn test_set_unconfirmed_duplicate_confirm_larger_slot_first() {
let smaller_duplicate_slot = 1;
let larger_duplicate_slot = 4;
let (mut progress, bank_forks) = setup_set_unconfirmed_and_confirmed_duplicate_slot_tests(
smaller_duplicate_slot,
larger_duplicate_slot,
);
let descendants = bank_forks.read().unwrap().descendants().clone();
let ancestors = bank_forks.read().unwrap().ancestors();
// Mark the larger duplicate slot as confirmed
progress.set_confirmed_duplicate_slot(
larger_duplicate_slot,
&ancestors.get(&larger_duplicate_slot).unwrap(),
&descendants.get(&larger_duplicate_slot).unwrap(),
);
// All slots should no longer have any unconfirmed duplicate ancestors
progress.set_confirmed_duplicate_slot(
larger_duplicate_slot,
&ancestors.get(&larger_duplicate_slot).unwrap(),
&descendants.get(&larger_duplicate_slot).unwrap(),
);
for slot in bank_forks.read().unwrap().banks().keys() {
// All slots <= the latest duplciate confirmed slot are ancestors of
// that slot, so they should all be marked duplicate confirmed
assert_eq!(
progress.is_duplicate_confirmed(*slot).unwrap(),
*slot <= larger_duplicate_slot
);
assert!(progress
.latest_unconfirmed_duplicate_ancestor(*slot)
.is_none());
}
}
}