change: Remove slot dependency from selection inherents (#1071)

Author: AmbientTea

Cargo.lock

@@ -12,6 +12,11 @@ Pallet Session new parameters should be: `type KeyDeposit = ();` and `type Curre
 * Updated partner-chains-smart-contracts (raw-scripts) dependency to v8.2.0. Not breaking.
 * `SessionManager` and `ShouldEndSession` implementations of `pallet-session-validator-management` rotate one additional
 session in order to make `pallet_session` use authorities with less delay.
+* Removed use of slots in `authority-selection-inherents`. `AriadneInherentDataProvider::new` now uses current timestamp
+and epoch duration to calculate current epoch.
+Since the block timestamp is not passed to the inherent data provider creation logic by Aura, the exact timestamp is not
+available during block verification, but it can be approximated based on the slot number. See the example impelementation
+in the demo node crate.
 
 ## Removed
 

changelog.md

@@ -105,10 +105,10 @@ where
 
 		let ariadne_data_provider = AriadneIDP::new(
 			client.as_ref(),
-			sc_slot_config,
+			sc_slot_config.epoch_duration().as_millis() as u64,
 			mc_epoch_config,
 			parent_hash,
-			*slot,
+			(*timestamp).as_millis(),
 			authority_selection_data_source.as_ref(),
 			mc_hash.mc_epoch(),
 		)
@@ -211,9 +211,11 @@ where
 			governed_map_data_source,
 			bridge_data_source,
 		} = self;
-		let CreateInherentDataConfig { mc_epoch_config, sc_slot_config, time_source, .. } = config;
+		let CreateInherentDataConfig { mc_epoch_config, sc_slot_config, .. } = config;
 
-		let timestamp = TimestampIDP::new(Timestamp::new(time_source.get_current_time_millis()));
+		let timestamp = TimestampIDP::new(Timestamp::new(
+			sc_slot_config.slot_start_time(verified_block_slot).unix_millis(),
+		));
 		let parent_header = client.expect_header(parent_hash)?;
 		let parent_slot = slot_from_predigest(&parent_header)?;
 		let mc_state_reference = McHashIDP::new_verification(
@@ -228,10 +230,10 @@ where
 
 		let ariadne_data_provider = AriadneIDP::new(
 			client.as_ref(),
-			sc_slot_config,
+			sc_slot_config.epoch_duration().as_millis() as u64,
 			mc_epoch_config,
 			parent_hash,
-			verified_block_slot,
+			(*timestamp).as_millis(),
 			authority_selection_data_source.as_ref(),
 			mc_state_reference.epoch,
 		)

demo/node/src/inherent_data.rs

@@ -24,15 +24,13 @@ plutus-datum-derive = { workspace = true }
 scale-info = { workspace = true }
 selection = { workspace = true }
 sidechain-domain = { workspace = true }
-sidechain-slots = { workspace = true, optional = true }
 sp-api = { workspace = true }
 sp-core = { workspace = true }
 sp-inherents = { workspace = true }
 sp-runtime = { workspace = true }
 sp-std = { workspace = true }
 thiserror = { workspace = true, optional = true }
 serde = { workspace = true }
-sp-consensus-slots = { workspace = true }
 
 [dev-dependencies]
 hex = { workspace = true }
@@ -49,9 +47,6 @@ std = [
 	"sidechain-domain/std",
 	"parity-scale-codec/std",
 	"serde/std",
-	"sidechain-slots",
-	"sidechain-slots/std",
-	"sidechain-slots/serde",
 	"sp-api/std",
 	"sp-session-validator-management/std",
 	"sp-session-validator-management/serde",

toolkit/committee-selection/authority-selection-inherents/Cargo.toml

@@ -9,7 +9,6 @@ use {
 	sidechain_domain::mainchain_epoch::MainchainEpochDerivation,
 	sidechain_domain::*,
 	sp_api::ProvideRuntimeApi,
-	sp_consensus_slots::Slot,
 	sp_inherents::{InherentData, InherentIdentifier},
 	sp_runtime::traits::Block as BlockT,
 	sp_session_validator_management::CommitteeMember as CommitteeMemberT,
@@ -19,7 +18,7 @@ use {
 };
 
 #[cfg(feature = "std")]
-pub use {sidechain_domain::mainchain_epoch::MainchainEpochConfig, sidechain_slots::ScSlotConfig};
+pub use sidechain_domain::mainchain_epoch::MainchainEpochConfig;
 
 #[derive(Clone, Debug, Encode, Decode)]
 /// Inherent data provider providing inputs for authority selection.
@@ -34,18 +33,18 @@ impl AriadneInherentDataProvider {
 	///
 	/// Parameters:
 	/// - `client`: runtime client capable of providing [SessionValidatorManagementApi] runtime API
-	/// - `sc_slot_config`: partner chain slot configuration
+	/// - `sc_epoch_duration_millis`: duration of the Partner Chain epoch in milliseconds
 	/// - `mc_epoch_config`: main chain epoch configuration
 	/// - `parent_hash`: parent hash of the current block
-	/// - `slot`: slot of the current block
+	/// - `timestamp_millis`: timestamp of current block in milliseconds
 	/// - `data_source`: data source implementing [AuthoritySelectionDataSource]
 	/// - `mc_reference_epoch`: latest stable mainchain epoch
 	pub async fn new<Block, CommitteeMember, T>(
 		client: &T,
-		sc_slot_config: &ScSlotConfig,
+		sc_epoch_duration_millis: u64,
 		mc_epoch_config: &MainchainEpochConfig,
 		parent_hash: <Block as BlockT>::Hash,
-		slot: Slot,
+		timestamp_millis: u64,
 		data_source: &(dyn AuthoritySelectionDataSource + Send + Sync),
 		mc_reference_epoch: McEpochNumber,
 	) -> Result<Self, InherentProviderCreationError>
@@ -64,10 +63,10 @@ impl AriadneInherentDataProvider {
 	{
 		let for_mc_epoch = mc_epoch_for_next_ariadne_cidp(
 			client,
-			sc_slot_config,
+			sc_epoch_duration_millis,
 			mc_epoch_config,
 			parent_hash,
-			slot,
+			timestamp_millis,
 		)?;
 
 		let data_epoch = data_source.data_epoch(for_mc_epoch).await?;
@@ -100,9 +99,6 @@ impl AriadneInherentDataProvider {
 #[derive(Debug, thiserror::Error)]
 /// Error type returned when creation of [AriadneInherentDataProvider] fails.
 pub enum InherentProviderCreationError {
-	/// Slot represents a timestamp bigger than of [u64::MAX].
-	#[error("Slot represents a timestamp bigger than of u64::MAX")]
-	SlotTooBig,
 	/// Couldn't convert timestamp to main chain epoch.
 	#[error("Couldn't convert timestamp to main chain epoch: {0}")]
 	McEpochDerivationError(#[from] sidechain_domain::mainchain_epoch::EpochDerivationError),
@@ -120,10 +116,10 @@ pub enum InherentProviderCreationError {
 #[cfg(feature = "std")]
 fn mc_epoch_for_next_ariadne_cidp<Block, CommitteeMember, T>(
 	client: &T,
-	sc_slot_config: &ScSlotConfig,
+	sc_epoch_duration_millis: u64,
 	epoch_config: &MainchainEpochConfig,
 	parent_hash: <Block as BlockT>::Hash,
-	slot: Slot,
+	timestamp_millis: u64,
 ) -> Result<McEpochNumber, InherentProviderCreationError>
 where
 	Block: BlockT,
@@ -138,34 +134,27 @@ where
 			ScEpochNumber,
 		>,
 {
+	use sp_core::offchain::Timestamp;
+
 	let next_unset_epoch = client.runtime_api().get_next_unset_epoch_number(parent_hash)?;
 
-	let for_sc_epoch_number = {
+	let for_timestamp = {
 		// A special case for the genesis committee (current epoch 0, next unset epoch 1).
 		// The genesis committee epoch is initialized with 0, so in the very first block we need to provide
-		// the epoch number based on the current slot number
+		// the epoch number based on the current timestamp
 		if next_unset_epoch == ScEpochNumber(1) {
-			sc_slot_config.epoch_number(slot)
+			// We first convert the timestamp to PC epoch number and then use the starting time of this
+			// to ensure that all timestamps within a PC epoch produce the same candidates. This is
+			// necessary in case the boundaries of PC epochs and MC epochs do not align.
+			let current_pc_epoch = timestamp_millis / sc_epoch_duration_millis;
+			current_pc_epoch * sc_epoch_duration_millis
 		} else {
-			next_unset_epoch
+			next_unset_epoch.0 * sc_epoch_duration_millis
 		}
 	};
 
-	sc_epoch_to_mc_epoch(for_sc_epoch_number, sc_slot_config, epoch_config)
-}
-
-#[cfg(feature = "std")]
-fn sc_epoch_to_mc_epoch(
-	sc_epoch: ScEpochNumber,
-	sc_slot_config: &ScSlotConfig,
-	epoch_config: &MainchainEpochConfig,
-) -> Result<McEpochNumber, InherentProviderCreationError> {
-	let timestamp = sc_slot_config
-		.epoch_start_time(sc_epoch)
-		.ok_or(InherentProviderCreationError::SlotTooBig)?;
-
 	epoch_config
-		.timestamp_to_mainchain_epoch(timestamp)
+		.timestamp_to_mainchain_epoch(Timestamp::from_unix_millis(for_timestamp))
 		.map_err(InherentProviderCreationError::McEpochDerivationError)
 }
 
@@ -205,24 +194,24 @@ mod tests {
 	use crate::ariadne_inherent_data_provider::AriadneInherentDataProvider;
 	use crate::mock::MockAuthoritySelectionDataSource;
 	use crate::runtime_api_mock::*;
-	use SlotDuration;
 	use sidechain_domain::mainchain_epoch::*;
-	use sidechain_slots::*;
 	use sp_core::H256;
 	use sp_core::offchain::Timestamp;
 
+	const TIMESTAMP: u64 = 400_000;
+
 	#[tokio::test]
 	async fn return_empty_ariadne_cidp_if_runtime_requests_too_new_epoch() {
 		// This is the epoch number that is too new
 		let next_unset_epoch_number = ScEpochNumber(42);
 		let mc_reference_epoch = McEpochNumber(1);
 		let empty_ariadne_idp = AriadneInherentDataProvider::new(
 			&client(next_unset_epoch_number),
-			&sc_slot_config(),
+			sc_epoch_duration_millis(),
 			&epoch_config(),
 			H256::zero(),
-			// This is the slot that will be used to calculate current_epoch_number
-			Slot::from(400u64),
+			// This is the timestamp that will be used to calculate current_epoch_number
+			TIMESTAMP,
 			&MockAuthoritySelectionDataSource::default(),
 			mc_reference_epoch,
 		)
@@ -241,11 +230,11 @@ mod tests {
 		let mc_reference_epoch = McEpochNumber(5);
 		let ariadne_idp = AriadneInherentDataProvider::new(
 			&client(next_unset_epoch_number),
-			&sc_slot_config(),
+			sc_epoch_duration_millis(),
 			&epoch_config(),
 			H256::zero(),
-			// This is the slot that will be used to calculate current_epoch_number
-			Slot::from(400u64),
+			// This is the timestamp that will be used to calculate current_epoch_number
+			TIMESTAMP,
 			&MockAuthoritySelectionDataSource::default()
 				.with_permissioned_candidates(vec![None, None, None, None, None])
 				.with_num_permissioned_candidates(3),
@@ -266,11 +255,11 @@ mod tests {
 		let mc_reference_epoch = McEpochNumber(5);
 		let ariadne_idp = AriadneInherentDataProvider::new(
 			&client(next_unset_epoch_number),
-			&sc_slot_config(),
+			sc_epoch_duration_millis(),
 			&epoch_config(),
 			H256::zero(),
-			// This is the slot that will be used to calculate current_epoch_number
-			Slot::from(400u64),
+			// This is the timestamp that will be used to calculate current_epoch_number
+			TIMESTAMP,
 			&MockAuthoritySelectionDataSource::default()
 				.with_permissioned_candidates(vec![None, None, None, None, None])
 				.with_num_permissioned_candidates(0),
@@ -282,11 +271,8 @@ mod tests {
 		assert!(ariadne_idp.unwrap().data.is_some());
 	}
 
-	fn sc_slot_config() -> ScSlotConfig {
-		ScSlotConfig {
-			slots_per_epoch: SlotsPerEpoch(10),
-			slot_duration: SlotDuration::from_millis(1000),
-		}
+	fn sc_epoch_duration_millis() -> u64 {
+		60 * 60 * 1000
 	}
 
 	fn client(next_unset_epoch_number: ScEpochNumber) -> TestApi {
@@ -297,11 +283,7 @@ mod tests {
 		MainchainEpochConfig {
 			first_epoch_timestamp_millis: Timestamp::from_unix_millis(0),
 			first_epoch_number: 0,
-			epoch_duration_millis: Duration::from_millis(
-				u64::from(sc_slot_config().slots_per_epoch.0)
-					* sc_slot_config().slot_duration.as_millis()
-					* 10,
-			),
+			epoch_duration_millis: Duration::from_millis(10 * sc_epoch_duration_millis()),
 			first_slot_number: 0,
 			slot_duration_millis: Duration::from_millis(1000),
 		}

toolkit/committee-selection/authority-selection-inherents/src/ariadne_inherent_data_provider.rs

@@ -7,7 +7,7 @@
 #[cfg(feature = "std")]
 pub mod runtime_api_client;
 
-use core::ops::Rem;
+use core::{ops::Rem, time::Duration};
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
 #[cfg(feature = "serde")]
@@ -86,6 +86,12 @@ pub struct ScSlotConfig {
 }
 
 impl ScSlotConfig {
+	#[cfg(feature = "std")]
+	/// Returns epoch duration
+	pub fn epoch_duration(&self) -> Duration {
+		self.slot_duration.as_duration() * self.slots_per_epoch.0
+	}
+
 	/// Returns the epoch number of `slot`
 	pub fn epoch_number(&self, slot: Slot) -> ScEpochNumber {
 		self.slots_per_epoch.epoch_number(slot)
@@ -101,6 +107,11 @@ impl ScSlotConfig {
 		Slot::from_timestamp(timestamp.into(), self.slot_duration)
 	}
 
+	/// Returns the start timestamp of `slot`
+	pub fn slot_start_time(&self, slot: Slot) -> Timestamp {
+		Timestamp::from_unix_millis(self.slot_duration.as_millis() * u64::from(slot))
+	}
+
 	/// Returns the start timestamp of `epoch`
 	pub fn epoch_start_time(&self, epoch: ScEpochNumber) -> Option<Timestamp> {
 		self.first_slot_number(epoch)