always broadcast tranche 0 assignments and add a delay before approval

node/core/approval-voting/src/approval_checking.rs

@@ -58,6 +58,8 @@ pub enum RequiredTranches {
 		/// event that there are some assignments that don't have corresponding approval votes. If this
 		/// is `None`, all assignments have approvals.
 		next_no_show: Option<Tick>,
+		/// The last tick at which a needed assignment was received.
+		last_assignment_tick: Option<Tick>,
 	},
 }
 
@@ -66,26 +68,30 @@ pub enum RequiredTranches {
 pub enum Check {
 	/// The candidate is unapproved.
 	Unapproved,
-	/// The candidate is approved, with the given amount of no-shows.
-	Approved(usize),
+	/// The candidate is approved, with the given amount of no-shows,
+	/// with the last counted assignment being received at the given
+	/// tick.
+	Approved(usize, Option<Tick>),
 	/// The candidate is approved by one third of all validators.
 	ApprovedOneThird,
 }
 
 impl Check {
-	/// Whether the candidate is approved.
-	pub fn is_approved(&self) -> bool {
+	/// Whether the candidate is approved and all relevant assignments
+	/// have at most the given assignment tick.
+	pub fn is_approved(&self, max_assignment_tick: Tick) -> bool {
 		match *self {
 			Check::Unapproved => false,
-			Check::Approved(_) => true,
+			Check::Approved(_, last_assignment_tick) =>
+				last_assignment_tick.map_or(true, |t| t <= max_assignment_tick),
 			Check::ApprovedOneThird => true,
 		}
 	}
 
 	/// The number of known no-shows in this computation.
 	pub fn known_no_shows(&self) -> usize {
 		match *self {
-			Check::Approved(n) => n,
+			Check::Approved(n, _) => n,
 			_ => 0,
 		}
 	}
@@ -107,7 +113,7 @@ pub fn check_approval(
 	match required {
 		RequiredTranches::Pending { .. } => Check::Unapproved,
 		RequiredTranches::All => Check::Unapproved,
-		RequiredTranches::Exact { needed, tolerated_missing, .. } => {
+		RequiredTranches::Exact { needed, tolerated_missing, last_assignment_tick, .. } => {
 			// whether all assigned validators up to `needed` less no_shows have approved.
 			// e.g. if we had 5 tranches and 1 no-show, we would accept all validators in
 			// tranches 0..=5 except for 1 approving. In that example, we also accept all
@@ -130,7 +136,7 @@ pub fn check_approval(
 			// that will surpass a minimum amount of checks.
 			// shouldn't typically go above, since all no-shows are supposed to be covered.
 			if n_approved + tolerated_missing >= n_assigned {
-				Check::Approved(tolerated_missing)
+				Check::Approved(tolerated_missing, last_assignment_tick)
 			} else {
 				Check::Unapproved
 			}
@@ -170,6 +176,8 @@ struct State {
 	uncovered: usize,
 	/// The next tick at which a no-show would occur, if any.
 	next_no_show: Option<Tick>,
+	/// The last tick at which a considered assignment was received.
+	last_assignment_tick: Option<Tick>,
 }
 
 impl State {
@@ -192,6 +200,7 @@ impl State {
 				needed: tranche,
 				tolerated_missing: self.covered,
 				next_no_show: self.next_no_show,
+				last_assignment_tick: self.last_assignment_tick,
 			}
 		}
 
@@ -226,6 +235,7 @@ impl State {
 		new_assignments: usize,
 		new_no_shows: usize,
 		next_no_show: Option<Tick>,
+		last_assignment_tick: Option<Tick>,
 	) -> State {
 		let new_covered = if self.depth == 0 {
 			new_assignments
@@ -246,6 +256,7 @@ impl State {
 		};
 		let uncovered = self.uncovered + new_no_shows;
 		let next_no_show = super::min_prefer_some(self.next_no_show, next_no_show);
+		let last_assignment_tick = std::cmp::max(self.last_assignment_tick, last_assignment_tick);
 
 		let (depth, covering, uncovered) = if covering == 0 {
 			if uncovered == 0 {
@@ -257,7 +268,15 @@ impl State {
 			(self.depth, covering, uncovered)
 		};
 
-		State { assignments, depth, covered, covering, uncovered, next_no_show }
+		State {
+			assignments,
+			depth,
+			covered,
+			covering,
+			uncovered,
+			next_no_show,
+			last_assignment_tick,
+		}
 	}
 }
 
@@ -356,6 +375,7 @@ pub fn tranches_to_approve(
 		covering: needed_approvals,
 		uncovered: 0,
 		next_no_show: None,
+		last_assignment_tick: None,
 	};
 
 	// The `ApprovalEntry` doesn't have any data for empty tranches. We still want to iterate over
@@ -384,6 +404,11 @@ pub fn tranches_to_approve(
 				.filter(|(v_index, _)| v_index.0 < n_validators as u32)
 				.count();
 
+			// Get the latest tick of valid validator assignments.
+			let last_assignment_tick = assignments.iter()
+				.map(|(_, t)| *t)
+				.max();
+
 			// count no-shows. An assignment is a no-show if there is no corresponding approval vote
 			// after a fixed duration.
 			//
@@ -397,7 +422,7 @@ pub fn tranches_to_approve(
 				drifted_tick_now,
 			);
 
-			let s = s.advance(n_assignments, no_shows, next_no_show);
+			let s = s.advance(n_assignments, no_shows, next_no_show, last_assignment_tick);
 			let output = s.output(tranche, needed_approvals, n_validators, no_show_duration);
 
 			*state = match output {
@@ -459,7 +484,7 @@ mod tests {
 				clock_drift: 0,
 			},
 		)
-		.is_approved());
+		.is_approved(Tick::max_value()));
 	}
 
 	#[test]
@@ -502,21 +527,36 @@ mod tests {
 		assert!(check_approval(
 			&candidate,
 			&approval_entry,
-			RequiredTranches::Exact { needed: 0, tolerated_missing: 0, next_no_show: None },
+			RequiredTranches::Exact {
+				needed: 0,
+				tolerated_missing: 0,
+				next_no_show: None,
+				last_assignment_tick: None
+			},
 		)
-		.is_approved());
+		.is_approved(Tick::max_value()));
 		assert!(!check_approval(
 			&candidate,
 			&approval_entry,
-			RequiredTranches::Exact { needed: 1, tolerated_missing: 0, next_no_show: None },
+			RequiredTranches::Exact {
+				needed: 1,
+				tolerated_missing: 0,
+				next_no_show: None,
+				last_assignment_tick: None
+			},
 		)
-		.is_approved());
+		.is_approved(Tick::max_value()));
 		assert!(check_approval(
 			&candidate,
 			&approval_entry,
-			RequiredTranches::Exact { needed: 1, tolerated_missing: 2, next_no_show: None },
+			RequiredTranches::Exact {
+				needed: 1,
+				tolerated_missing: 2,
+				next_no_show: None,
+				last_assignment_tick: None
+			},
 		)
-		.is_approved());
+		.is_approved(Tick::max_value()));
 	}
 
 	#[test]
@@ -556,8 +596,12 @@ mod tests {
 		}
 		.into();
 
-		let exact_all =
-			RequiredTranches::Exact { needed: 10, tolerated_missing: 0, next_no_show: None };
+		let exact_all = RequiredTranches::Exact {
+			needed: 10,
+			tolerated_missing: 0,
+			next_no_show: None,
+			last_assignment_tick: None,
+		};
 
 		let pending_all = RequiredTranches::Pending {
 			considered: 5,
@@ -566,20 +610,27 @@ mod tests {
 			clock_drift: 12,
 		};
 
-		assert!(!check_approval(&candidate, &approval_entry, RequiredTranches::All,).is_approved());
+		assert!(!check_approval(&candidate, &approval_entry, RequiredTranches::All,)
+			.is_approved(Tick::max_value()));
 
-		assert!(!check_approval(&candidate, &approval_entry, exact_all.clone(),).is_approved());
+		assert!(!check_approval(&candidate, &approval_entry, exact_all.clone(),)
+			.is_approved(Tick::max_value()));
 
-		assert!(!check_approval(&candidate, &approval_entry, pending_all.clone(),).is_approved());
+		assert!(!check_approval(&candidate, &approval_entry, pending_all.clone(),)
+			.is_approved(Tick::max_value()));
 
 		// This creates a set of 4/10 approvals, which is always an approval.
 		candidate.mark_approval(ValidatorIndex(3));
 
-		assert!(check_approval(&candidate, &approval_entry, RequiredTranches::All,).is_approved());
+		assert!(check_approval(&candidate, &approval_entry, RequiredTranches::All,)
+			.is_approved(Tick::max_value()));
 
-		assert!(check_approval(&candidate, &approval_entry, exact_all,).is_approved());
+		assert!(
+			check_approval(&candidate, &approval_entry, exact_all,).is_approved(Tick::max_value())
+		);
 
-		assert!(check_approval(&candidate, &approval_entry, pending_all,).is_approved());
+		assert!(check_approval(&candidate, &approval_entry, pending_all,)
+			.is_approved(Tick::max_value()));
 	}
 
 	#[test]
@@ -617,7 +668,12 @@ mod tests {
 				no_show_duration,
 				needed_approvals,
 			),
-			RequiredTranches::Exact { needed: 1, tolerated_missing: 0, next_no_show: None },
+			RequiredTranches::Exact {
+				needed: 1,
+				tolerated_missing: 0,
+				next_no_show: None,
+				last_assignment_tick: Some(1)
+			},
 		);
 	}
 
@@ -820,6 +876,7 @@ mod tests {
 				needed: 1,
 				tolerated_missing: 0,
 				next_no_show: Some(block_tick + no_show_duration + 1),
+				last_assignment_tick: Some(block_tick + 1),
 			},
 		);
 
@@ -838,6 +895,7 @@ mod tests {
 				needed: 2,
 				tolerated_missing: 1,
 				next_no_show: Some(block_tick + 2 * no_show_duration + 2),
+				last_assignment_tick: Some(block_tick + no_show_duration + 2),
 			},
 		);
 
@@ -905,7 +963,12 @@ mod tests {
 				no_show_duration,
 				needed_approvals,
 			),
-			RequiredTranches::Exact { needed: 2, tolerated_missing: 1, next_no_show: None },
+			RequiredTranches::Exact {
+				needed: 2,
+				tolerated_missing: 1,
+				next_no_show: None,
+				last_assignment_tick: Some(block_tick + no_show_duration + 2)
+			},
 		);
 
 		// Even though tranche 2 has 2 validators, it only covers 1 no-show.
@@ -943,7 +1006,12 @@ mod tests {
 				no_show_duration,
 				needed_approvals,
 			),
-			RequiredTranches::Exact { needed: 3, tolerated_missing: 2, next_no_show: None },
+			RequiredTranches::Exact {
+				needed: 3,
+				tolerated_missing: 2,
+				next_no_show: None,
+				last_assignment_tick: Some(block_tick + no_show_duration + 2),
+			},
 		);
 	}
 
@@ -1256,43 +1324,50 @@ mod tests {
 			exp_next_no_show: None,
 		})
 	}
-}
 
-#[test]
-fn depth_0_covering_not_treated_as_such() {
-	let state = State {
-		assignments: 0,
-		depth: 0,
-		covered: 0,
-		covering: 10,
-		uncovered: 0,
-		next_no_show: None,
-	};
-
-	assert_eq!(
-		state.output(0, 10, 10, 20),
-		RequiredTranches::Pending {
-			considered: 0,
+	#[test]
+	fn depth_0_covering_not_treated_as_such() {
+		let state = State {
+			assignments: 0,
+			depth: 0,
+			covered: 0,
+			covering: 10,
+			uncovered: 0,
 			next_no_show: None,
-			maximum_broadcast: DelayTranche::max_value(),
-			clock_drift: 0,
-		},
-	);
-}
+			last_assignment_tick: None,
+		};
 
-#[test]
-fn depth_0_issued_as_exact_even_when_all() {
-	let state = State {
-		assignments: 10,
-		depth: 0,
-		covered: 0,
-		covering: 0,
-		uncovered: 0,
-		next_no_show: None,
-	};
+		assert_eq!(
+			state.output(0, 10, 10, 20),
+			RequiredTranches::Pending {
+				considered: 0,
+				next_no_show: None,
+				maximum_broadcast: DelayTranche::max_value(),
+				clock_drift: 0,
+			},
+		);
+	}
 
-	assert_eq!(
-		state.output(0, 10, 10, 20),
-		RequiredTranches::Exact { needed: 0, tolerated_missing: 0, next_no_show: None },
-	);
+	#[test]
+	fn depth_0_issued_as_exact_even_when_all() {
+		let state = State {
+			assignments: 10,
+			depth: 0,
+			covered: 0,
+			covering: 0,
+			uncovered: 0,
+			next_no_show: None,
+			last_assignment_tick: None,
+		};
+
+		assert_eq!(
+			state.output(0, 10, 10, 20),
+			RequiredTranches::Exact {
+				needed: 0,
+				tolerated_missing: 0,
+				next_no_show: None,
+				last_assignment_tick: None
+			},
+		);
+	}
 }