Do not alter original epoch changes node on epoch skip

client/consensus/sassafras/src/block_import.rs

@@ -165,20 +165,23 @@ where
 				old_epoch_changes = Some((*epoch_changes).clone());
 
 				let mut viable_epoch = epoch_changes
-					.viable_epoch_mut(&epoch_descriptor, |slot| {
+					.viable_epoch(&epoch_descriptor, |slot| {
 						Epoch::genesis(&self.genesis_config, slot)
 					})
 					.ok_or_else(|| {
 						ConsensusError::ClientImport(Error::<Block>::FetchEpoch(parent_hash).into())
-					})?;
+					})?
+					.into_cloned();
 
 				if viable_epoch.as_ref().end_slot() <= slot {
 					// Some epochs must have been skipped as our current slot fits outside the
 					// current epoch. We will figure out which is the first skipped epoch and we
 					// will partially re-use its data for this "recovery" epoch.
 					let epoch_data = viable_epoch.as_mut();
-					let slot_idx = u64::from(slot) - u64::from(epoch_data.start_slot);
-					let skipped_epochs = slot_idx / self.genesis_config.epoch_duration;
+					let skipped_epochs =
+						(*slot - *epoch_data.start_slot) / epoch_data.config.epoch_duration;
+					let original_epoch_idx = epoch_data.epoch_index;
+
 					// NOTE: notice that we are only updating a local copy of the `Epoch`, this
 					// makes it so that when we insert the next epoch into `EpochChanges` below
 					// (after incrementing it), it will use the correct epoch index and start slot.
@@ -190,13 +193,12 @@ where
 					// requiring to update `start_slot` to the correct value.
 					epoch_data.epoch_index += skipped_epochs;
 					epoch_data.start_slot = Slot::from(
-						u64::from(epoch_data.start_slot) +
-							skipped_epochs * self.genesis_config.epoch_duration,
+						*epoch_data.start_slot + skipped_epochs * epoch_data.config.epoch_duration,
 					);
 					log::warn!(
 						target: "sassafras",
-						"🌳 Detected {} skipped epochs, starting recovery epoch {}",
-						skipped_epochs, epoch_data.epoch_index
+						"🌳 Epoch(s) skipped from {} to {}",
+						 original_epoch_idx, epoch_data.epoch_index
 					);
 				}
 

client/consensus/sassafras/src/tests.rs

@@ -396,16 +396,19 @@ fn allows_to_skip_epochs() {
 	assert_eq!(data.epoch_index, 1);
 	assert_eq!(data.start_slot, Slot::from(7));
 
-	// First block in E1 (B7) announces E2 (config then stolen by E3)
+	// First block in E1 (B7) announces E2
+	// NOTE: config has been "magically" used by E3 without altering epoch node values.
+	// This will break as soon as our assumptions about how fork-tree works are not met anymore
+	// (and this is a good thing)
 	let data = epoch_changes
 		.epoch(&EpochIdentifier {
 			position: EpochIdentifierPosition::Regular,
 			hash: blocks[6],
 			number: 7,
 		})
 		.unwrap();
-	assert_eq!(data.epoch_index, 3);
-	assert_eq!(data.start_slot, Slot::from(19));
+	assert_eq!(data.epoch_index, 2);
+	assert_eq!(data.start_slot, Slot::from(13));
 
 	// First block in E3 (B8) announced E4.
 	let data = epoch_changes