Simplify persisting tower local cluster tests

[carllin]

Problem

Optimistic confirmation tests are needlessly convoluted.

The optimistic confirmation is not necessary for the core of the test which is to guarantee tower prevents bad voting behavior across restarts.

Summary of Changes

1. Remove the need for optimistic confirmation
2. Simplify the forking logic
3. Remove the need for validator D

Ran each test 100 times, no failures

Fixes #

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (dd51e61) to head (41ae0a5).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master     #875     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         851      851             
  Lines      231471   231471             
=========================================
- Hits       189534   189507     -27     
- Misses      41937    41964     +27     

[wen-coding]

We don't have a D now. By the way, why did we need D before but not anymore?

[carllin]

It's been a while, but I think this comment is a clue:

agave/local-cluster/tests/local_cluster.rs

Line 3166 in 41ae0a5

// also important so `C` doesn't run into NoPropagatedConfirmation errors on making its first forked

I think it comes from a time before we added this change:

agave/local-cluster/tests/local_cluster.rs

Lines 3196 to 3199 in 41ae0a5

	// C should not produce any blocks at this time
	validator_configs[2].fixed_leader_schedule = Some(FixedSchedule {
	leader_schedule: Arc::new(c_leader_schedule),
	});

to prevent C from making blocks before the base slot forking point.

and when we relied on waiting for validators A and C to vote on and OC some slot > next_slot_on_a, where that slot could have been produced by validator C

In this scenario, in order for C to reliably restart and make another fork, I think it would have needed to pass a propagated check, which implies seeing the vote from A in gossip, which is no longer necessary

[wen-coding]

do you mean: it may create different versions of blocks for some blocks already created on a different fork?

[carllin]

yup, clarified!

[wen-coding]

I'm confused, we are now checking:

last_vote >= 1

is it possible we only have S2 and S3 (no S0 and S1)?

[carllin]

Correct, the only important slots are S2 and S3 because S2 is the last common ancestor slot slot and S3 is the fork that gets deleted, but still exists/doesn't exist in tower depending on which test we run.

You're right this S0, S1, S2, S3 stuff is quite confusing, I deleted all of this and replaced it with just the variable names base_slot, next_slot_on_a for clarity