Port unconfirmed duplicate tracking logic from ProgressMap to ForkChoice

[carllin]

Problem

Keeping the duplicate/duplicate confirmed tracking logic in ProgressMap is unwieldy for a number of reasons:

1. Requires a separate update in cluster_slot_state_verifier to update the ProgressMap, and then separately mark certain forks valid/invalid in ForkChoice:

   solana/core/src/cluster_slot_state_verifier.rs

   Lines 212 to 217 in 18ec675

   	progress.set_unconfirmed_duplicate_slot(
   	slot,
   	descendants.get(&slot).unwrap_or(&HashSet::default()),
   	);
   	fork_choice.mark_fork_invalid_candidate(&(slot, bank_frozen_hash));
   	}

2. Updating the ProgressMap for duplicate/duplicate confirmed tracking requires passing in ancestors/descendants:

   solana/core/src/progress_map.rs

   Line 477 in 18ec675

   pub fn set_unconfirmed_duplicate_slot(&mut self, slot: Slot, descendants: &HashSet<u64>) {

   because it doesn't know about fork structure. This means ancestors/descendants have to be passed along to cluster_slot_state_verifier::check_slot_agrees_with_cluster()

   solana/core/src/cluster_slot_state_verifier.rs

   Lines 242 to 243 in 18ec675

   	ancestors: &HashMap<Slot, HashSet<Slot>>,
   	descendants: &HashMap<Slot, HashSet<Slot>>,

   and a variety of ReplayStage functions that call down to that function

Furthermore, there are correctness issues that can arise because we track all the duplicate ancestor information in the ProgressMap and not ForkChoice. These stem from the fact that the fork choice only knows that a specific block was marked valid/invalid, but doesn't know if descendants of that block are valid/invalid. For example, consider the following order of events:

Given a fork 0 -> 1 -> 2 -> 3

1. Fork 1 is marked duplicate. Currently, we set the is_candidate flag to false

   solana/core/src/heaviest_subtree_fork_choice.rs

   Line 819 in 18ec675

   fork_info.is_candidate = false;

   and then removes the entire subtree from fork choice.
2. Fork 2 is marked duplicate, and removes its entire subtree from fork choice, similar to 1) above.
3. Fork 1 is marked valid. At this point, without searching all of its ancestors, fork 3 has no idea whether it is part of fork choice because its own is_candidate flag is not set.

Now 3) is not an issue when considering the overall best slot returned by fork choice:

solana/core/src/heaviest_subtree_fork_choice.rs

Lines 175 to 177 in 18ec675

	pub fn best_overall_slot(&self) -> SlotHashKey {
	self.best_slot(&self.root).unwrap()
	}

, because that looks at the best slot field at the root node, and the root node contains aggregate information from the invalid fork 2. However, functions like heaviest_slot_on_same_voted_fork():

solana/core/src/heaviest_subtree_fork_choice.rs

Line 688 in 18ec675

let heaviest_slot_hash_on_same_voted_fork = self.best_slot(&last_voted_slot_hash);

, look at the best slot field of a node that is potentially in the subtree of 2, and didn't contain the aggregate information that 2 was marked invalid.

Summary of Changes

Port duplicate fork/duplicate confirmed status tracking and tests to fork choice

Fixes #

[jstarry]

Can you also mention that this will be set to the ForkInfo's own slot if it's a duplicate?

[codecov]

Codecov Report

Merging #17779 (5ce142b) into master (afafa62) will increase coverage by 0.0%.
The diff coverage is 97.7%.

@@           Coverage Diff            @@
##           master   #17779    +/-   ##
========================================
  Coverage    82.6%    82.6%            
========================================
  Files         431      431            
  Lines      121044   121178   +134     
========================================
+ Hits       100035   100167   +132     
- Misses      21009    21011     +2     

[jstarry]

lgtm

Pull request has been modified.