Warp sync part II (#9284)

* Gap sync

* Gap epoch test

* Simplified network requests

* Update client/db/src/utils.rs

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

* Fixed v1 migration and added some comments

* Next epoch is always regular

* Removed fork tree change

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Added a comment and converted assert to error

Co-authored-by: cheme <emericchevalier.pro@gmail.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/client/consensus/epochs/src/lib.rs

@@ -78,11 +78,11 @@ where
 ///
 /// Once an epoch is created, it must have a known `start_slot` and `end_slot`, which cannot be
 /// changed. Consensus engine may modify any other data in the epoch, if needed.
-pub trait Epoch {
+pub trait Epoch: std::fmt::Debug {
 	/// Descriptor for the next epoch.
 	type NextEpochDescriptor;
 	/// Type of the slot number.
-	type Slot: Ord + Copy;
+	type Slot: Ord + Copy + std::fmt::Debug;
 
 	/// The starting slot of the epoch.
 	fn start_slot(&self) -> Self::Slot;
@@ -228,7 +228,7 @@ impl<Hash, Number, E: Epoch> ViableEpochDescriptor<Hash, Number, E> {
 }
 
 /// Persisted epoch stored in EpochChanges.
-#[derive(Clone, Encode, Decode)]
+#[derive(Clone, Encode, Decode, Debug)]
 pub enum PersistedEpoch<E: Epoch> {
 	/// Genesis persisted epoch data. epoch_0, epoch_1.
 	Genesis(E, E),
@@ -246,8 +246,23 @@ impl<'a, E: Epoch> From<&'a PersistedEpoch<E>> for PersistedEpochHeader<E> {
 	}
 }
 
+impl<E: Epoch> PersistedEpoch<E> {
+	/// Map the epoch to a different type using a conversion function.
+	pub fn map<B, F, Hash, Number>(self, h: &Hash, n: &Number, f: &mut F) -> PersistedEpoch<B>
+	where
+		B: Epoch<Slot = E::Slot>,
+		F: FnMut(&Hash, &Number, E) -> B,
+	{
+		match self {
+			PersistedEpoch::Genesis(epoch_0, epoch_1) =>
+				PersistedEpoch::Genesis(f(h, n, epoch_0), f(h, n, epoch_1)),
+			PersistedEpoch::Regular(epoch_n) => PersistedEpoch::Regular(f(h, n, epoch_n)),
+		}
+	}
+}
+
 /// Persisted epoch header stored in ForkTree.
-#[derive(Encode, Decode, PartialEq, Eq)]
+#[derive(Encode, Decode, PartialEq, Eq, Debug)]
 pub enum PersistedEpochHeader<E: Epoch> {
 	/// Genesis persisted epoch header. epoch_0, epoch_1.
 	Genesis(EpochHeader<E>, EpochHeader<E>),
@@ -264,6 +279,25 @@ impl<E: Epoch> Clone for PersistedEpochHeader<E> {
 	}
 }
 
+impl<E: Epoch> PersistedEpochHeader<E> {
+	/// Map the epoch header to a different type.
+	pub fn map<B>(self) -> PersistedEpochHeader<B>
+	where
+		B: Epoch<Slot = E::Slot>,
+	{
+		match self {
+			PersistedEpochHeader::Genesis(epoch_0, epoch_1) => PersistedEpochHeader::Genesis(
+				EpochHeader { start_slot: epoch_0.start_slot, end_slot: epoch_0.end_slot },
+				EpochHeader { start_slot: epoch_1.start_slot, end_slot: epoch_1.end_slot },
+			),
+			PersistedEpochHeader::Regular(epoch_n) => PersistedEpochHeader::Regular(EpochHeader {
+				start_slot: epoch_n.start_slot,
+				end_slot: epoch_n.end_slot,
+			}),
+		}
+	}
+}
+
 /// A fresh, incremented epoch to import into the underlying fork-tree.
 ///
 /// Create this with `ViableEpoch::increment`.
@@ -279,6 +313,106 @@ impl<E: Epoch> AsRef<E> for IncrementedEpoch<E> {
 	}
 }
 
+/// A pair of epochs for the gap block download validation.
+/// Block gap is created after the warp sync is complete. Blocks
+/// are imported both at the tip of the chain and at the start of the gap.
+/// This holds a pair of epochs that are required to validate headers
+/// at the start of the gap. Since gap download does not allow forks we don't
+/// need to keep a tree of epochs.
+#[derive(Clone, Encode, Decode, Debug)]
+pub struct GapEpochs<Hash, Number, E: Epoch> {
+	current: (Hash, Number, PersistedEpoch<E>),
+	next: Option<(Hash, Number, E)>,
+}
+
+impl<Hash, Number, E> GapEpochs<Hash, Number, E>
+where
+	Hash: Copy + PartialEq + std::fmt::Debug,
+	Number: Copy + PartialEq + std::fmt::Debug,
+	E: Epoch,
+{
+	/// Check if given slot matches one of the gap epochs.
+	/// Returns epoch identifier if it does.
+	fn matches(
+		&self,
+		slot: E::Slot,
+	) -> Option<(Hash, Number, EpochHeader<E>, EpochIdentifierPosition)> {
+		match &self.current {
+			(_, _, PersistedEpoch::Genesis(epoch_0, _))
+				if slot >= epoch_0.start_slot() && slot < epoch_0.end_slot() =>
+				return Some((
+					self.current.0,
+					self.current.1,
+					epoch_0.into(),
+					EpochIdentifierPosition::Genesis0,
+				)),
+			(_, _, PersistedEpoch::Genesis(_, epoch_1))
+				if slot >= epoch_1.start_slot() && slot < epoch_1.end_slot() =>
+				return Some((
+					self.current.0,
+					self.current.1,
+					epoch_1.into(),
+					EpochIdentifierPosition::Genesis1,
+				)),
+			(_, _, PersistedEpoch::Regular(epoch_n))
+				if slot >= epoch_n.start_slot() && slot < epoch_n.end_slot() =>
+				return Some((
+					self.current.0,
+					self.current.1,
+					epoch_n.into(),
+					EpochIdentifierPosition::Regular,
+				)),
+			_ => {},
+		};
+		match &self.next {
+			Some((h, n, epoch_n)) if slot >= epoch_n.start_slot() && slot < epoch_n.end_slot() =>
+				Some((*h, *n, epoch_n.into(), EpochIdentifierPosition::Regular)),
+			_ => None,
+		}
+	}
+
+	/// Returns epoch data if it matches given identifier.
+	pub fn epoch(&self, id: &EpochIdentifier<Hash, Number>) -> Option<&E> {
+		match (&self.current, &self.next) {
+			((h, n, e), _) if h == &id.hash && n == &id.number => match e {
+				PersistedEpoch::Genesis(ref epoch_0, _)
+					if id.position == EpochIdentifierPosition::Genesis0 =>
+					Some(epoch_0),
+				PersistedEpoch::Genesis(_, ref epoch_1)
+					if id.position == EpochIdentifierPosition::Genesis1 =>
+					Some(epoch_1),
+				PersistedEpoch::Regular(ref epoch_n)
+					if id.position == EpochIdentifierPosition::Regular =>
+					Some(epoch_n),
+				_ => None,
+			},
+			(_, Some((h, n, e)))
+				if h == &id.hash &&
+					n == &id.number && id.position == EpochIdentifierPosition::Regular =>
+				Some(e),
+			_ => None,
+		}
+	}
+
+	/// Import a new gap epoch, potentially replacing an old epoch.
+	fn import(&mut self, slot: E::Slot, hash: Hash, number: Number, epoch: E) -> Result<(), E> {
+		match (&mut self.current, &mut self.next) {
+			((_, _, PersistedEpoch::Genesis(_, epoch_1)), _) if slot == epoch_1.end_slot() => {
+				self.next = Some((hash, number, epoch));
+				Ok(())
+			},
+			(_, Some((_, _, epoch_n))) if slot == epoch_n.end_slot() => {
+				let (cur_h, cur_n, cur_epoch) =
+					self.next.take().expect("Already matched as `Some`");
+				self.current = (cur_h, cur_n, PersistedEpoch::Regular(cur_epoch));
+				self.next = Some((hash, number, epoch));
+				Ok(())
+			},
+			_ => Err(epoch),
+		}
+	}
+}
+
 /// Tree of all epoch changes across all *seen* forks. Data stored in tree is
 /// the hash and block number of the block signaling the epoch change, and the
 /// epoch that was signalled at that block.
@@ -294,10 +428,14 @@ impl<E: Epoch> AsRef<E> for IncrementedEpoch<E> {
 /// same DAG entry, pinned to a specific block #1.
 ///
 /// Further epochs (epoch_2, ..., epoch_n) each get their own entry.
-#[derive(Clone, Encode, Decode)]
+///
+/// Also maintains a pair of epochs for the start of the gap,
+/// as long as there's an active gap download after a warp sync.
+#[derive(Clone, Encode, Decode, Debug)]
 pub struct EpochChanges<Hash, Number, E: Epoch> {
 	inner: ForkTree<Hash, Number, PersistedEpochHeader<E>>,
 	epochs: BTreeMap<(Hash, Number), PersistedEpoch<E>>,
+	gap: Option<GapEpochs<Hash, Number, E>>,
 }
 
 // create a fake header hash which hasn't been included in the chain.
@@ -315,14 +453,14 @@ where
 	Number: Ord,
 {
 	fn default() -> Self {
-		EpochChanges { inner: ForkTree::new(), epochs: BTreeMap::new() }
+		EpochChanges { inner: ForkTree::new(), epochs: BTreeMap::new(), gap: None }
 	}
 }
 
 impl<Hash, Number, E: Epoch> EpochChanges<Hash, Number, E>
 where
-	Hash: PartialEq + Ord + AsRef<[u8]> + AsMut<[u8]> + Copy,
-	Number: Ord + One + Zero + Add<Output = Number> + Sub<Output = Number> + Copy,
+	Hash: PartialEq + Ord + AsRef<[u8]> + AsMut<[u8]> + Copy + std::fmt::Debug,
+	Number: Ord + One + Zero + Add<Output = Number> + Sub<Output = Number> + Copy + std::fmt::Debug,
 {
 	/// Create a new epoch change.
 	pub fn new() -> Self {
@@ -335,6 +473,11 @@ where
 		self.inner.rebalance()
 	}
 
+	/// Clear gap epochs if any.
+	pub fn clear_gap(&mut self) {
+		self.gap = None;
+	}
+
 	/// Map the epoch changes from one storing data to a different one.
 	pub fn map<B, F>(self, mut f: F) -> EpochChanges<Hash, Number, B>
 	where
@@ -342,31 +485,15 @@ where
 		F: FnMut(&Hash, &Number, E) -> B,
 	{
 		EpochChanges {
-			inner: self.inner.map(&mut |_, _, header| match header {
-				PersistedEpochHeader::Genesis(epoch_0, epoch_1) => PersistedEpochHeader::Genesis(
-					EpochHeader { start_slot: epoch_0.start_slot, end_slot: epoch_0.end_slot },
-					EpochHeader { start_slot: epoch_1.start_slot, end_slot: epoch_1.end_slot },
-				),
-				PersistedEpochHeader::Regular(epoch_n) =>
-					PersistedEpochHeader::Regular(EpochHeader {
-						start_slot: epoch_n.start_slot,
-						end_slot: epoch_n.end_slot,
-					}),
+			inner: self.inner.map(&mut |_, _, header: PersistedEpochHeader<E>| header.map()),
+			gap: self.gap.map(|GapEpochs { current: (h, n, header), next }| GapEpochs {
+				current: (h, n, header.map(&h, &n, &mut f)),
+				next: next.map(|(h, n, e)| (h, n, f(&h, &n, e))),
 			}),
 			epochs: self
 				.epochs
 				.into_iter()
-				.map(|((hash, number), epoch)| {
-					let bepoch = match epoch {
-						PersistedEpoch::Genesis(epoch_0, epoch_1) => PersistedEpoch::Genesis(
-							f(&hash, &number, epoch_0),
-							f(&hash, &number, epoch_1),
-						),
-						PersistedEpoch::Regular(epoch_n) =>
-							PersistedEpoch::Regular(f(&hash, &number, epoch_n)),
-					};
-					((hash, number), bepoch)
-				})
+				.map(|((hash, number), epoch)| ((hash, number), epoch.map(&hash, &number, &mut f)))
 				.collect(),
 		}
 	}
@@ -402,6 +529,9 @@ where
 
 	/// Get a reference to an epoch with given identifier.
 	pub fn epoch(&self, id: &EpochIdentifier<Hash, Number>) -> Option<&E> {
+		if let Some(e) = &self.gap.as_ref().and_then(|gap| gap.epoch(id)) {
+			return Some(e)
+		}
 		self.epochs.get(&(id.hash, id.number)).and_then(|v| match v {
 			PersistedEpoch::Genesis(ref epoch_0, _)
 				if id.position == EpochIdentifierPosition::Genesis0 =>
@@ -537,6 +667,15 @@ where
 			return Ok(Some(ViableEpochDescriptor::UnimportedGenesis(slot)))
 		}
 
+		if let Some(gap) = &self.gap {
+			if let Some((hash, number, hdr, position)) = gap.matches(slot) {
+				return Ok(Some(ViableEpochDescriptor::Signaled(
+					EpochIdentifier { position, hash, number },
+					hdr,
+				)))
+			}
+		}
+
 		// We want to find the deepest node in the tree which is an ancestor
 		// of our block and where the start slot of the epoch was before the
 		// slot of our block. The genesis special-case doesn't need to look
@@ -598,13 +737,30 @@ where
 	) -> Result<(), fork_tree::Error<D::Error>> {
 		let is_descendent_of =
 			descendent_of_builder.build_is_descendent_of(Some((hash, parent_hash)));
-		let header = PersistedEpochHeader::<E>::from(&epoch.0);
+		let slot = epoch.as_ref().start_slot();
+		let IncrementedEpoch(mut epoch) = epoch;
+		let header = PersistedEpochHeader::<E>::from(&epoch);
+
+		if let Some(gap) = &mut self.gap {
+			if let PersistedEpoch::Regular(e) = epoch {
+				epoch = match gap.import(slot, hash.clone(), number.clone(), e) {
+					Ok(()) => return Ok(()),
+					Err(e) => PersistedEpoch::Regular(e),
+				}
+			}
+		} else if !self.epochs.is_empty() && matches!(epoch, PersistedEpoch::Genesis(_, _)) {
+			// There's a genesis epoch imported when we already have an active epoch.
+			// This happens after the warp sync as the ancient blocks download start.
+			// We need to start tracking gap epochs here.
+			self.gap = Some(GapEpochs { current: (hash, number, epoch), next: None });
+			return Ok(())
+		}
 
 		let res = self.inner.import(hash, number, header, &is_descendent_of);
 
 		match res {
 			Ok(_) | Err(fork_tree::Error::Duplicate) => {
-				self.epochs.insert((hash, number), epoch.0);
+				self.epochs.insert((hash, number), epoch);
 				Ok(())
 			},
 			Err(e) => Err(e),
@@ -916,4 +1072,112 @@ mod tests {
 			assert!(epoch_for_x_child_before_genesis.is_none());
 		}
 	}
+
+	#[test]
+	fn gap_epochs_advance() {
+		// 0 - 1 - 2 - 3 - .... 42 - 43
+		let is_descendent_of = |base: &Hash, block: &Hash| -> Result<bool, TestError> {
+			match (base, *block) {
+				(b"0", _) => Ok(true),
+				(b"1", b) => Ok(b == *b"0"),
+				(b"2", b) => Ok(b == *b"1"),
+				(b"3", b) => Ok(b == *b"2"),
+				_ => Ok(false),
+			}
+		};
+
+		let duration = 100;
+
+		let make_genesis = |slot| Epoch { start_slot: slot, duration };
+
+		let mut epoch_changes = EpochChanges::new();
+		let next_descriptor = ();
+
+		let epoch42 = Epoch { start_slot: 42, duration: 100 };
+		let epoch43 = Epoch { start_slot: 43, duration: 100 };
+		epoch_changes.reset(*b"0", *b"1", 4200, epoch42, epoch43);
+		assert!(epoch_changes.gap.is_none());
+
+		// Import a new genesis epoch, this should crate the gap.
+		let genesis_epoch_a_descriptor = epoch_changes
+			.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 100)
+			.unwrap()
+			.unwrap();
+
+		let incremented_epoch = epoch_changes
+			.viable_epoch(&genesis_epoch_a_descriptor, &make_genesis)
+			.unwrap()
+			.increment(next_descriptor.clone());
+
+		epoch_changes
+			.import(&is_descendent_of, *b"1", 1, *b"0", incremented_epoch)
+			.unwrap();
+		assert!(epoch_changes.gap.is_some());
+
+		let genesis_epoch = epoch_changes
+			.epoch_descriptor_for_child_of(&is_descendent_of, b"0", 0, 100)
+			.unwrap()
+			.unwrap();
+
+		assert_eq!(genesis_epoch, ViableEpochDescriptor::UnimportedGenesis(100));
+
+		// Import more epochs and check that gap advances.
+		let import_epoch_1 =
+			epoch_changes.viable_epoch(&genesis_epoch, &make_genesis).unwrap().increment(());
+
+		let epoch_1 = import_epoch_1.as_ref().clone();
+		epoch_changes
+			.import(&is_descendent_of, *b"1", 1, *b"0", import_epoch_1)
+			.unwrap();
+		let genesis_epoch_data = epoch_changes.epoch_data(&genesis_epoch, &make_genesis).unwrap();
+		let end_slot = genesis_epoch_data.end_slot();
+		let x = epoch_changes
+			.epoch_data_for_child_of(&is_descendent_of, b"1", 1, end_slot, &make_genesis)
+			.unwrap()
+			.unwrap();
+
+		assert_eq!(x, epoch_1);
+		assert_eq!(epoch_changes.gap.as_ref().unwrap().current.0, *b"1");
+		assert!(epoch_changes.gap.as_ref().unwrap().next.is_none());
+
+		let epoch_1_desriptor = epoch_changes
+			.epoch_descriptor_for_child_of(&is_descendent_of, b"1", 1, end_slot)
+			.unwrap()
+			.unwrap();
+		let epoch_1 = epoch_changes.epoch_data(&epoch_1_desriptor, &make_genesis).unwrap();
+		let import_epoch_2 = epoch_changes
+			.viable_epoch(&epoch_1_desriptor, &make_genesis)
+			.unwrap()
+			.increment(());
+		let epoch_2 = import_epoch_2.as_ref().clone();
+		epoch_changes
+			.import(&is_descendent_of, *b"2", 2, *b"1", import_epoch_2)
+			.unwrap();
+
+		let end_slot = epoch_1.end_slot();
+		let x = epoch_changes
+			.epoch_data_for_child_of(&is_descendent_of, b"2", 2, end_slot, &make_genesis)
+			.unwrap()
+			.unwrap();
+		assert_eq!(epoch_changes.gap.as_ref().unwrap().current.0, *b"1");
+		assert_eq!(epoch_changes.gap.as_ref().unwrap().next.as_ref().unwrap().0, *b"2");
+		assert_eq!(x, epoch_2);
+
+		let epoch_2_desriptor = epoch_changes
+			.epoch_descriptor_for_child_of(&is_descendent_of, b"2", 2, end_slot)
+			.unwrap()
+			.unwrap();
+		let import_epoch_3 = epoch_changes
+			.viable_epoch(&epoch_2_desriptor, &make_genesis)
+			.unwrap()
+			.increment(());
+		epoch_changes
+			.import(&is_descendent_of, *b"3", 3, *b"2", import_epoch_3)
+			.unwrap();
+
+		assert_eq!(epoch_changes.gap.as_ref().unwrap().current.0, *b"2");
+
+		epoch_changes.clear_gap();
+		assert!(epoch_changes.gap.is_none());
+	}
 }

substrate/client/consensus/epochs/src/migration.rs

@@ -30,9 +30,19 @@ pub struct EpochChangesV0<Hash, Number, E: Epoch> {
 	inner: ForkTree<Hash, Number, PersistedEpoch<E>>,
 }
 
-/// Type alias for legacy definition of epoch changes.
-pub type EpochChangesForV0<Block, Epoch> =
+/// Legacy definition of epoch changes.
+#[derive(Clone, Encode, Decode)]
+pub struct EpochChangesV1<Hash, Number, E: Epoch> {
+	inner: ForkTree<Hash, Number, PersistedEpochHeader<E>>,
+	epochs: BTreeMap<(Hash, Number), PersistedEpoch<E>>,
+}
+
+/// Type alias for v0 definition of epoch changes.
+pub type EpochChangesV0For<Block, Epoch> =
 	EpochChangesV0<<Block as BlockT>::Hash, NumberFor<Block>, Epoch>;
+/// Type alias for v1 and v2 definition of epoch changes.
+pub type EpochChangesV1For<Block, Epoch> =
+	EpochChangesV1<<Block as BlockT>::Hash, NumberFor<Block>, Epoch>;
 
 impl<Hash, Number, E: Epoch> EpochChangesV0<Hash, Number, E>
 where
@@ -54,6 +64,17 @@ where
 			header
 		});
 
-		EpochChanges { inner, epochs }
+		EpochChanges { inner, epochs, gap: None }
+	}
+}
+
+impl<Hash, Number, E: Epoch> EpochChangesV1<Hash, Number, E>
+where
+	Hash: PartialEq + Ord + Copy,
+	Number: Ord + Copy,
+{
+	/// Migrate the type into current epoch changes definition.
+	pub fn migrate(self) -> EpochChanges<Hash, Number, E> {
+		EpochChanges { inner: self.inner, epochs: self.epochs, gap: None }
 	}
 }