Further refactory

client/consensus/sassafras/src/authorship.rs

@@ -23,14 +23,17 @@ use super::*;
 use sp_consensus_sassafras::{
 	digests::PreDigest,
 	vrf::{make_slot_transcript_data, make_ticket_transcript_data},
-	AuthorityId, Slot, Ticket, TicketInfo,
+	AuthorityId, Slot, Ticket, TicketAux,
 };
 use sp_core::{twox_64, ByteArray};
 
 /// Get secondary authority index for the given epoch and slot.
-pub(crate) fn secondary_authority_index(slot: Slot, config: &SassafrasConfiguration) -> u64 {
-	u64::from_le_bytes((config.randomness, slot).using_encoded(twox_64)) %
-		config.authorities.len() as u64
+pub(crate) fn secondary_authority_index(
+	slot: Slot,
+	config: &SassafrasConfiguration,
+) -> AuthorityIndex {
+	u64::from_le_bytes((config.randomness, slot).using_encoded(twox_64)) as AuthorityIndex %
+		config.authorities.len() as AuthorityIndex
 }
 
 /// Try to claim an epoch slot.
@@ -42,22 +45,21 @@ fn claim_slot(
 	keystore: &SyncCryptoStorePtr,
 ) -> Option<(PreDigest, AuthorityId)> {
 	let config = &epoch.config;
-	let (authority_index, ticket_info) = match ticket {
+	let (authority_idx, ticket_aux) = match ticket {
 		Some(ticket) => {
 			log::debug!(target: "sassafras", "🌳 [TRY PRIMARY]");
-			let ticket_info = epoch.tickets_info.get(&ticket)?.clone();
+			let (authority_idx, ticket_aux) = epoch.tickets_aux.get(&ticket)?.clone();
 			log::debug!(target: "sassafras", "🌳 Ticket = [ticket: {:02x?}, auth: {}, attempt: {}]",
-                &ticket.as_bytes()[0..8], ticket_info.authority_index, ticket_info.attempt);
-			let idx = ticket_info.authority_index as u64;
-			(idx, Some(ticket_info))
+                &ticket.as_bytes()[0..8], authority_idx, ticket_aux.attempt);
+			(authority_idx, Some(ticket_aux))
 		},
 		None => {
 			log::debug!(target: "sassafras", "🌳 [TRY SECONDARY]");
 			(secondary_authority_index(slot, config), None)
 		},
 	};
 
-	let authority_id = config.authorities.get(authority_index as usize).map(|auth| &auth.0)?;
+	let authority_id = config.authorities.get(authority_idx as usize).map(|auth| &auth.0)?;
 
 	let transcript_data = make_slot_transcript_data(&config.randomness, slot, epoch.epoch_index);
 	let signature = SyncCryptoStore::sr25519_vrf_sign(
@@ -70,19 +72,19 @@ fn claim_slot(
 	.flatten()?;
 
 	let pre_digest = PreDigest {
-		authority_index: authority_index as u32,
+		authority_idx,
 		slot,
 		vrf_output: VRFOutput(signature.output),
 		vrf_proof: VRFProof(signature.proof.clone()),
-		ticket_info,
+		ticket_aux,
 	};
 
 	Some((pre_digest, authority_id.clone()))
 }
 
 /// Generate the tickets for the given epoch.
-/// Tickets additional information (i.e. `TicketInfo`) will be stored within the `Epoch`
-/// structure. The additional information will be used during epoch to claim slots.
+/// Tickets additional information will be stored within the `Epoch` structure.
+/// The additional information will be used later during session to claim slots.
 pub fn generate_epoch_tickets(epoch: &mut Epoch, keystore: &SyncCryptoStorePtr) -> Vec<Ticket> {
 	let config = &epoch.config;
 	let max_attempts = config.threshold_params.attempts_number;
@@ -99,7 +101,7 @@ pub fn generate_epoch_tickets(epoch: &mut Epoch, keystore: &SyncCryptoStorePtr)
 	log::debug!(target: "sassafras", "🌳 Tickets threshold: {:032x}", threshold);
 
 	let authorities = config.authorities.iter().enumerate().map(|(index, a)| (index, &a.0));
-	for (authority_index, authority_id) in authorities {
+	for (authority_idx, authority_id) in authorities {
 		if !SyncCryptoStore::has_keys(&**keystore, &[(authority_id.to_raw_vec(), AuthorityId::ID)])
 		{
 			continue
@@ -124,19 +126,16 @@ pub fn generate_epoch_tickets(epoch: &mut Epoch, keystore: &SyncCryptoStorePtr)
 				return None
 			}
 
-			let ticket_info = TicketInfo {
-				attempt: attempt as u32,
-				authority_index: authority_index as u32,
-				proof: VRFProof(signature.proof),
-			};
+			let ticket_aux =
+				TicketAux { attempt: attempt as u32, proof: VRFProof(signature.proof) };
 
-			Some((ticket, ticket_info))
+			Some((ticket, ticket_aux))
 		};
 
 		for attempt in 0..max_attempts {
-			if let Some((ticket, ticket_info)) = make_ticket(attempt) {
+			if let Some((ticket, ticket_aux)) = make_ticket(attempt) {
 				tickets.push(ticket);
-				epoch.tickets_info.insert(ticket, ticket_info);
+				epoch.tickets_aux.insert(ticket, (authority_idx as AuthorityIndex, ticket_aux));
 			}
 		}
 	}
@@ -420,7 +419,7 @@ async fn tickets_worker<B, C, SC>(
 			epoch_changes
 				.shared_data()
 				.epoch_mut(&epoch_identifier)
-				.map(|epoch| epoch.tickets_info.clear());
+				.map(|epoch| epoch.tickets_aux.clear());
 		}
 	}
 }

client/consensus/sassafras/src/block_import.rs

@@ -143,8 +143,7 @@ where
 			let epoch_descriptor = intermediate.epoch_descriptor;
 			let first_in_epoch = parent_slot < epoch_descriptor.start_slot();
 
-			let added_weight = pre_digest.ticket_info.is_some() as u32;
-			let total_weight = parent_weight + added_weight;
+			let total_weight = parent_weight + pre_digest.ticket_aux.is_some() as u32;
 
 			// Search for this all the time so we can reject unexpected announcements.
 			let next_epoch_digest = find_next_epoch_digest::<Block>(&block.header)

client/consensus/sassafras/src/lib.rs

@@ -85,9 +85,9 @@ pub use sp_consensus_sassafras::{
 	digests::{CompatibleDigestItem, ConsensusLog, NextEpochDescriptor, PreDigest},
 	inherents::SassafrasInherentData,
 	vrf::{make_slot_transcript, make_ticket_transcript},
-	AuthorityId, AuthorityPair, AuthoritySignature, SassafrasApi, SassafrasAuthorityWeight,
-	SassafrasConfiguration, SassafrasEpochConfiguration, Ticket, TicketInfo, VRFOutput, VRFProof,
-	SASSAFRAS_ENGINE_ID, VRF_OUTPUT_LENGTH, VRF_PROOF_LENGTH,
+	AuthorityId, AuthorityIndex, AuthorityPair, AuthoritySignature, SassafrasApi,
+	SassafrasAuthorityWeight, SassafrasConfiguration, SassafrasEpochConfiguration, Ticket,
+	TicketAux, VRFOutput, VRFProof, SASSAFRAS_ENGINE_ID, VRF_OUTPUT_LENGTH, VRF_PROOF_LENGTH,
 };
 
 mod authorship;
@@ -194,8 +194,8 @@ pub struct Epoch {
 	pub start_slot: Slot,
 	/// Epoch configuration
 	pub config: SassafrasConfiguration,
-	/// Tickets metadata.
-	pub tickets_info: BTreeMap<Ticket, TicketInfo>,
+	/// Tickets auxiliary data.
+	pub tickets_aux: BTreeMap<Ticket, (AuthorityIndex, TicketAux)>,
 }
 
 impl EpochT for Epoch {
@@ -214,7 +214,7 @@ impl EpochT for Epoch {
 			epoch_index: self.epoch_index + 1,
 			start_slot: self.start_slot + config.epoch_duration,
 			config,
-			tickets_info: BTreeMap::new(),
+			tickets_aux: BTreeMap::new(),
 		}
 	}
 
@@ -235,7 +235,7 @@ impl Epoch {
 			epoch_index: 0,
 			start_slot: slot,
 			config: config.clone(),
-			tickets_info: BTreeMap::new(),
+			tickets_aux: BTreeMap::new(),
 		}
 	}
 }
@@ -276,11 +276,11 @@ fn find_pre_digest<B: BlockT>(header: &B::Header) -> Result<PreDigest, Error<B>>
 		let vrf_output = VRFOutput::try_from([0; VRF_OUTPUT_LENGTH]).expect(PROOF);
 		let vrf_proof = VRFProof::try_from([0; VRF_PROOF_LENGTH]).expect(PROOF);
 		return Ok(PreDigest {
-			authority_index: 0,
+			authority_idx: 0,
 			slot: 0.into(),
 			vrf_output,
 			vrf_proof,
-			ticket_info: None,
+			ticket_aux: None,
 		})
 	}
 

client/consensus/sassafras/src/verification.rs

@@ -38,9 +38,9 @@ pub struct VerificationParams<'a, B: 'a + BlockT> {
 }
 
 pub struct VerifiedHeaderInfo {
+	pub authority_id: AuthorityId,
 	pub pre_digest: DigestItem,
 	pub seal: DigestItem,
-	pub author: AuthorityId,
 }
 
 /// Check a header has been signed by the right key. If the slot is too far in
@@ -63,8 +63,8 @@ pub fn check_header<B: BlockT + Sized>(
 		return Ok(CheckedHeader::Deferred(header, pre_digest.slot))
 	}
 
-	let author = match config.authorities.get(pre_digest.authority_index as usize) {
-		Some(author) => author.0.clone(),
+	let authority_id = match config.authorities.get(pre_digest.authority_idx as usize) {
+		Some(authority_id) => authority_id.0.clone(),
 		None => return Err(sassafras_err(Error::SlotAuthorNotFound)),
 	};
 
@@ -80,55 +80,50 @@ pub fn check_header<B: BlockT + Sized>(
 		.ok_or_else(|| sassafras_err(Error::HeaderBadSeal(header.hash())))?;
 
 	let pre_hash = header.hash();
-	if !AuthorityPair::verify(&signature, &pre_hash, &author) {
+	if !AuthorityPair::verify(&signature, &pre_hash, &authority_id) {
 		return Err(sassafras_err(Error::BadSignature(pre_hash)))
 	}
 
 	// Check authorship method and claim
 
-	match (&ticket, &pre_digest.ticket_info) {
-		(Some(ticket), Some(ticket_info)) => {
+	match (&ticket, &pre_digest.ticket_aux) {
+		(Some(ticket), Some(ticket_aux)) => {
 			log::debug!(target: "sassafras", "🌳 checking primary");
-			if ticket_info.authority_index != pre_digest.authority_index {
-				// TODO-SASS-P2 ... we can eventually remove auth index from ticket info
-				log::error!(target: "sassafras", "🌳 Wrong primary authority index");
-			}
 			let transcript =
-				make_ticket_transcript(&config.randomness, ticket_info.attempt, epoch.epoch_index);
-			schnorrkel::PublicKey::from_bytes(author.as_slice())
-				.and_then(|p| p.vrf_verify(transcript, &ticket, &ticket_info.proof))
+				make_ticket_transcript(&config.randomness, ticket_aux.attempt, epoch.epoch_index);
+			schnorrkel::PublicKey::from_bytes(authority_id.as_slice())
+				.and_then(|p| p.vrf_verify(transcript, &ticket, &ticket_aux.proof))
 				.map_err(|s| sassafras_err(Error::VRFVerificationFailed(s)))?;
 		},
 		(None, None) => {
 			log::debug!(target: "sassafras", "🌳 checking secondary");
 			let idx = authorship::secondary_authority_index(pre_digest.slot, config);
-			if idx != pre_digest.authority_index as u64 {
-				log::error!(target: "sassafras", "🌳 Wrong secondary authority index");
+			if idx != pre_digest.authority_idx {
+				log::error!(target: "sassafras", "🌳 Bad secondary authority index");
+				return Err(Error::SlotAuthorNotFound)
 			}
 		},
 		(Some(_), None) => {
 			log::warn!(target: "sassafras", "🌳 Unexpected secondary authoring mechanism");
-			// TODO-SASS-P2: maybe we can use a different error variant
 			return Err(Error::UnexpectedAuthoringMechanism)
 		},
 		(None, Some(_)) => {
 			log::warn!(target: "sassafras", "🌳 Unexpected primary authoring mechanism");
-			// TODO-SASS-P2: maybe we will use a different error variant
 			return Err(Error::UnexpectedAuthoringMechanism)
 		},
 	}
 
 	// Check slot-vrf proof
 
 	let transcript = make_slot_transcript(&config.randomness, pre_digest.slot, epoch.epoch_index);
-	schnorrkel::PublicKey::from_bytes(author.as_slice())
+	schnorrkel::PublicKey::from_bytes(authority_id.as_slice())
 		.and_then(|p| p.vrf_verify(transcript, &pre_digest.vrf_output, &pre_digest.vrf_proof))
 		.map_err(|s| sassafras_err(Error::VRFVerificationFailed(s)))?;
 
 	let info = VerifiedHeaderInfo {
+		authority_id,
 		pre_digest: CompatibleDigestItem::sassafras_pre_digest(pre_digest),
 		seal,
-		author,
 	};
 
 	Ok(CheckedHeader::Checked(header, info))
@@ -368,7 +363,7 @@ where
 						slot_now,
 						slot,
 						&block.header,
-						&verified_info.author,
+						&verified_info.authority_id,
 						&block.origin,
 					)
 					.await

frame/sassafras/src/lib.rs

@@ -375,7 +375,7 @@ pub mod pallet {
 				// Current slot should be less than half of epoch duration.
 				let epoch_duration = T::EpochDuration::get();
 
-				if Self::current_slot_epoch_index() >= epoch_duration / 2 {
+				if Self::current_slot_index() >= epoch_duration / 2 {
 					log::warn!(
 						target: "sassafras::runtime",
 						"🌳 Timeout to propose tickets, bailing out.",
@@ -443,14 +443,16 @@ impl<T: Config> Pallet<T> {
 		// The exception is for block 1: the genesis has slot 0, so we treat epoch 0 as having
 		// started at the slot of block 1. We want to use the same randomness and validator set as
 		// signalled in the genesis, so we don't rotate the epoch.
-		now != One::one() && Self::current_slot_epoch_index() >= T::EpochDuration::get()
+		now != One::one() && Self::current_slot_index() >= T::EpochDuration::get()
 	}
 
-	fn current_slot_epoch_index() -> u64 {
-		Self::slot_epoch_index(CurrentSlot::<T>::get())
+	/// Current slot index with respect to current epoch.
+	fn current_slot_index() -> u64 {
+		Self::slot_index(CurrentSlot::<T>::get())
 	}
 
-	fn slot_epoch_index(slot: Slot) -> u64 {
+	/// Slot index with respect to current epoch.
+	fn slot_index(slot: Slot) -> u64 {
 		if *GenesisSlot::<T>::get() == 0 {
 			return 0
 		}
@@ -638,7 +640,7 @@ impl<T: Config> Pallet<T> {
 	pub fn slot_ticket(slot: Slot) -> Option<Ticket> {
 		let epoch_idx = EpochIndex::<T>::get();
 		let duration = T::EpochDuration::get();
-		let mut slot_idx = Self::slot_epoch_index(slot);
+		let mut slot_idx = Self::slot_index(slot);
 		let mut tickets_meta = TicketsMeta::<T>::get();
 
 		let get_ticket_idx = |slot_idx| {

frame/sassafras/src/tests.rs

@@ -123,7 +123,7 @@ fn on_first_block_after_genesis() {
 		assert_eq!(Sassafras::current_slot(), start_slot);
 		assert_eq!(Sassafras::epoch_index(), 0);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 0);
+		assert_eq!(Sassafras::current_slot_index(), 0);
 		assert_eq!(Sassafras::randomness(), [0; 32]);
 		assert_eq!(NextRandomness::<Test>::get(), [0; 32]);
 		assert_eq!(RandomnessAccumulator::<Test>::get(), [0; 32]);
@@ -138,7 +138,7 @@ fn on_first_block_after_genesis() {
 		assert_eq!(Sassafras::current_slot(), start_slot);
 		assert_eq!(Sassafras::epoch_index(), 0);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 0);
+		assert_eq!(Sassafras::current_slot_index(), 0);
 		assert_eq!(Sassafras::randomness(), [0; 32]);
 		assert_eq!(NextRandomness::<Test>::get(), [0; 32]);
 		assert_eq!(
@@ -188,7 +188,7 @@ fn on_normal_block() {
 		assert_eq!(Sassafras::current_slot(), start_slot + 1);
 		assert_eq!(Sassafras::epoch_index(), 0);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 1);
+		assert_eq!(Sassafras::current_slot_index(), 1);
 		assert_eq!(Sassafras::randomness(), [0; 32]);
 		assert_eq!(NextRandomness::<Test>::get(), [0; 32]);
 		assert_eq!(
@@ -206,7 +206,7 @@ fn on_normal_block() {
 		assert_eq!(Sassafras::current_slot(), start_slot + 1);
 		assert_eq!(Sassafras::epoch_index(), 0);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 1);
+		assert_eq!(Sassafras::current_slot_index(), 1);
 		assert_eq!(Sassafras::randomness(), [0; 32]);
 		assert_eq!(NextRandomness::<Test>::get(), [0; 32]);
 		assert_eq!(
@@ -244,7 +244,7 @@ fn epoch_change_block() {
 		assert_eq!(Sassafras::current_slot(), start_slot + epoch_duration);
 		assert_eq!(Sassafras::epoch_index(), 1);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot + epoch_duration);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 0);
+		assert_eq!(Sassafras::current_slot_index(), 0);
 		assert_eq!(Sassafras::randomness(), [0; 32],);
 		assert_eq!(
 			NextRandomness::<Test>::get(),
@@ -265,7 +265,7 @@ fn epoch_change_block() {
 		assert_eq!(Sassafras::current_slot(), start_slot + epoch_duration);
 		assert_eq!(Sassafras::epoch_index(), 1);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot + epoch_duration);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 0);
+		assert_eq!(Sassafras::current_slot_index(), 0);
 		assert_eq!(Sassafras::randomness(), [0; 32]);
 		assert_eq!(
 			NextRandomness::<Test>::get(),
@@ -416,7 +416,7 @@ fn block_skips_epochs() {
 		assert_eq!(Sassafras::current_slot(), start_slot + offset);
 		assert_eq!(Sassafras::epoch_index(), 3);
 		assert_eq!(Sassafras::current_epoch_start(), start_slot + offset);
-		assert_eq!(Sassafras::current_slot_epoch_index(), 0);
+		assert_eq!(Sassafras::current_slot_index(), 0);
 
 		// Tickets were discarded
 		let meta = TicketsMeta::<Test>::get();