From 431429fffbce9f0e9e7ddaddf63dcf6ad639ea39 Mon Sep 17 00:00:00 2001
From: Davide Galassi <davxy@datawok.net>
Date: Wed, 23 Nov 2022 08:10:37 +0100
Subject: [PATCH] Prevent epochs pruning while finalizing blocks on epoch 0
 (#12758)

* Prevent epochs pruning while on epoch 0
---
 client/consensus/epochs/src/lib.rs | 88 +++++++++++++++++++++++++++++-
 1 file changed, 85 insertions(+), 3 deletions(-)

diff --git a/client/consensus/epochs/src/lib.rs b/client/consensus/epochs/src/lib.rs
index f8b6253ef2353..c213a49b8e4e4 100644
--- a/client/consensus/epochs/src/lib.rs
+++ b/client/consensus/epochs/src/lib.rs
@@ -518,13 +518,13 @@ where
 		let is_descendent_of = descendent_of_builder.build_is_descendent_of(None);
 
 		let predicate = |epoch: &PersistedEpochHeader<E>| match *epoch {
-			PersistedEpochHeader::Genesis(ref epoch_0, _) => epoch_0.start_slot <= slot,
+			PersistedEpochHeader::Genesis(_, ref epoch_1) => epoch_1.start_slot <= slot,
 			PersistedEpochHeader::Regular(ref epoch_n) => epoch_n.start_slot <= slot,
 		};
 
-		// prune any epochs which could not be _live_ as of the children of the
+		// Prune any epochs which could not be _live_ as of the children of the
 		// finalized block, i.e. re-root the fork tree to the oldest ancestor of
-		// (hash, number) where epoch.end_slot() >= finalized_slot
+		// (hash, number) where `epoch.start_slot() <= finalized_slot`.
 		let removed = self.inner.prune(hash, &number, &is_descendent_of, &predicate)?;
 
 		for (hash, number, _) in removed {
@@ -1197,6 +1197,88 @@ mod tests {
 		assert_eq!(nodes, vec![b"B", b"C", b"F", b"G"]);
 	}
 
+	#[test]
+	fn near_genesis_prune_works() {
+		// [X]: announces next epoch change (i.e. adds a node in the epoch changes tree)
+		//
+		// 0--[A]--B--C--D--E--[G]--H--I--J--K--[L]
+		//                  +
+		//                  \--[F]
+
+		let is_descendent_of = |base: &Hash, block: &Hash| -> Result<bool, TestError> {
+			match (block, base) {
+				| (b"A", b"0") |
+				(b"B", b"0" | b"A") |
+				(b"C", b"0" | b"A" | b"B") |
+				(b"D", b"0" | b"A" | b"B" | b"C") |
+				(b"E", b"0" | b"A" | b"B" | b"C" | b"D") |
+				(b"F", b"0" | b"A" | b"B" | b"C" | b"D" | b"E") |
+				(b"G", b"0" | b"A" | b"B" | b"C" | b"D" | b"E") |
+				(b"H", b"0" | b"A" | b"B" | b"C" | b"D" | b"E" | b"G") |
+				(b"I", b"0" | b"A" | b"B" | b"C" | b"D" | b"E" | b"G" | b"H") |
+				(b"J", b"0" | b"A" | b"B" | b"C" | b"D" | b"E" | b"G" | b"H" | b"I") |
+				(b"K", b"0" | b"A" | b"B" | b"C" | b"D" | b"E" | b"G" | b"H" | b"I" | b"J") |
+				(
+					b"L",
+					b"0" | b"A" | b"B" | b"C" | b"D" | b"E" | b"G" | b"H" | b"I" | b"J" | b"K",
+				) => Ok(true),
+				_ => Ok(false),
+			}
+		};
+
+		let mut epoch_changes = EpochChanges::new();
+
+		let epoch = Epoch { start_slot: 278183811, duration: 5 };
+		let epoch = PersistedEpoch::Genesis(epoch.clone(), epoch.increment(()));
+
+		epoch_changes
+			.import(&is_descendent_of, *b"A", 1, Default::default(), IncrementedEpoch(epoch))
+			.unwrap();
+
+		let import_at = |epoch_changes: &mut EpochChanges<_, _, Epoch>,
+		                 slot,
+		                 hash: &Hash,
+		                 number,
+		                 parent_hash,
+		                 parent_number| {
+			let make_genesis = |slot| Epoch { start_slot: slot, duration: 5 };
+			// Get epoch descriptor valid for 'slot'
+			let epoch_descriptor = epoch_changes
+				.epoch_descriptor_for_child_of(&is_descendent_of, parent_hash, parent_number, slot)
+				.unwrap()
+				.unwrap();
+			// Increment it
+			let next_epoch_desc = epoch_changes
+				.viable_epoch(&epoch_descriptor, &make_genesis)
+				.unwrap()
+				.increment(());
+			// Assign it to hash/number
+			epoch_changes
+				.import(&is_descendent_of, *hash, number, *parent_hash, next_epoch_desc)
+				.unwrap();
+		};
+
+		// Should not prune anything
+		epoch_changes.prune_finalized(&is_descendent_of, b"C", 3, 278183813).unwrap();
+
+		import_at(&mut epoch_changes, 278183816, b"G", 6, b"E", 5);
+		import_at(&mut epoch_changes, 278183816, b"F", 6, b"E", 5);
+
+		// Should not prune anything since we are on epoch0
+		epoch_changes.prune_finalized(&is_descendent_of, b"C", 3, 278183813).unwrap();
+		let mut list: Vec<_> = epoch_changes.inner.iter().map(|e| e.0).collect();
+		list.sort();
+		assert_eq!(list, vec![b"A", b"F", b"G"]);
+
+		import_at(&mut epoch_changes, 278183821, b"L", 11, b"K", 10);
+
+		// Should prune any fork of our ancestor not in the canonical chain (i.e. "F")
+		epoch_changes.prune_finalized(&is_descendent_of, b"J", 9, 278183819).unwrap();
+		let mut list: Vec<_> = epoch_changes.inner.iter().map(|e| e.0).collect();
+		list.sort();
+		assert_eq!(list, vec![b"A", b"G", b"L"]);
+	}
+
 	/// Test that ensures that the gap is not enabled when we import multiple genesis blocks.
 	#[test]
 	fn gap_is_not_enabled_when_multiple_genesis_epochs_are_imported() {