Correct BABE randomness by calculating InOut bytes directly in pallet (#5876)

* vrf: remove Raw* types

* babe: remove Raw* types

* pallet-babe: switch representation of RawVRFOutput to Randomness

* pallet-babe: calculate inout within the pallet

* Remove make_transcript duplication

* Bump spec version

* Fix frame tests

* and_then -> map

* Always enable u64_backend

* Fix nostd compile

* fix import: should not use std

* Remove unused definition of RawVRFOutput

* Remove unused import of RuntimeDebug

Co-authored-by: Gavin Wood <gavin@parity.io>

substrate/frame/babe/src/lib.rs

@@ -34,13 +34,14 @@ use sp_staking::{
 	SessionIndex,
 	offence::{Offence, Kind},
 };
+use sp_application_crypto::Public;
 
 use codec::{Encode, Decode};
 use sp_inherents::{InherentIdentifier, InherentData, ProvideInherent, MakeFatalError};
 use sp_consensus_babe::{
 	BABE_ENGINE_ID, ConsensusLog, BabeAuthorityWeight, SlotNumber,
 	inherents::{INHERENT_IDENTIFIER, BabeInherentData},
-	digests::{NextEpochDescriptor, RawPreDigest},
+	digests::{NextEpochDescriptor, PreDigest},
 };
 use sp_consensus_vrf::schnorrkel;
 pub use sp_consensus_babe::{AuthorityId, VRF_OUTPUT_LENGTH, RANDOMNESS_LENGTH, PUBLIC_KEY_LENGTH};
@@ -102,7 +103,7 @@ impl EpochChangeTrigger for SameAuthoritiesForever {
 
 const UNDER_CONSTRUCTION_SEGMENT_LENGTH: usize = 256;
 
-type MaybeVrf = Option<schnorrkel::RawVRFOutput>;
+type MaybeRandomness = Option<schnorrkel::Randomness>;
 
 decl_storage! {
 	trait Store for Module<T: Trait> as Babe {
@@ -147,11 +148,11 @@ decl_storage! {
 		/// We reset all segments and return to `0` at the beginning of every
 		/// epoch.
 		SegmentIndex build(|_| 0): u32;
-		UnderConstruction: map hasher(twox_64_concat) u32 => Vec<schnorrkel::RawVRFOutput>;
+		UnderConstruction: map hasher(twox_64_concat) u32 => Vec<schnorrkel::Randomness>;
 
 		/// Temporary value (cleared at block finalization) which is `Some`
 		/// if per-block initialization has already been called for current block.
-		Initialized get(fn initialized): Option<MaybeVrf>;
+		Initialized get(fn initialized): Option<MaybeRandomness>;
 
 		/// How late the current block is compared to its parent.
 		///
@@ -194,8 +195,8 @@ decl_module! {
 			// that this block was the first in a new epoch, the changeover logic has
 			// already occurred at this point, so the under-construction randomness
 			// will only contain outputs from the right epoch.
-			if let Some(Some(vrf_output)) = Initialized::take() {
-				Self::deposit_vrf_output(&vrf_output);
+			if let Some(Some(randomness)) = Initialized::take() {
+				Self::deposit_randomness(&randomness);
 			}
 
 			// remove temporary "environment" entry from storage
@@ -238,7 +239,7 @@ impl<T: Trait> FindAuthor<u32> for Module<T> {
 	{
 		for (id, mut data) in digests.into_iter() {
 			if id == BABE_ENGINE_ID {
-				let pre_digest: RawPreDigest = RawPreDigest::decode(&mut data).ok()?;
+				let pre_digest: PreDigest = PreDigest::decode(&mut data).ok()?;
 				return Some(pre_digest.authority_index())
 			}
 		}
@@ -415,17 +416,17 @@ impl<T: Trait> Module<T> {
 		<frame_system::Module<T>>::deposit_log(log.into())
 	}
 
-	fn deposit_vrf_output(vrf_output: &schnorrkel::RawVRFOutput) {
+	fn deposit_randomness(randomness: &schnorrkel::Randomness) {
 		let segment_idx = <SegmentIndex>::get();
 		let mut segment = <UnderConstruction>::get(&segment_idx);
 		if segment.len() < UNDER_CONSTRUCTION_SEGMENT_LENGTH {
 			// push onto current segment: not full.
-			segment.push(*vrf_output);
+			segment.push(*randomness);
 			<UnderConstruction>::insert(&segment_idx, &segment);
 		} else {
 			// move onto the next segment and update the index.
 			let segment_idx = segment_idx + 1;
-			<UnderConstruction>::insert(&segment_idx, &vec![vrf_output.clone()]);
+			<UnderConstruction>::insert(&segment_idx, &vec![randomness.clone()]);
 			<SegmentIndex>::put(&segment_idx);
 		}
 	}
@@ -438,18 +439,18 @@ impl<T: Trait> Module<T> {
 			return;
 		}
 
-		let maybe_pre_digest: Option<RawPreDigest> = <frame_system::Module<T>>::digest()
+		let maybe_pre_digest: Option<PreDigest> = <frame_system::Module<T>>::digest()
 			.logs
 			.iter()
 			.filter_map(|s| s.as_pre_runtime())
 			.filter_map(|(id, mut data)| if id == BABE_ENGINE_ID {
-				RawPreDigest::decode(&mut data).ok()
+				PreDigest::decode(&mut data).ok()
 			} else {
 				None
 			})
 			.next();
 
-		let maybe_vrf = maybe_pre_digest.and_then(|digest| {
+		let maybe_randomness: Option<schnorrkel::Randomness> = maybe_pre_digest.and_then(|digest| {
 			// on the first non-zero block (i.e. block #1)
 			// this is where the first epoch (epoch #0) actually starts.
 			// we need to adjust internal storage accordingly.
@@ -478,17 +479,38 @@ impl<T: Trait> Module<T> {
 			Lateness::<T>::put(lateness);
 			CurrentSlot::put(current_slot);
 
-			if let RawPreDigest::Primary(primary) = digest {
+			if let PreDigest::Primary(primary) = digest {
 				// place the VRF output into the `Initialized` storage item
 				// and it'll be put onto the under-construction randomness
 				// later, once we've decided which epoch this block is in.
-				Some(primary.vrf_output)
+				//
+				// Reconstruct the bytes of VRFInOut using the authority id.
+				Authorities::get()
+					.get(primary.authority_index as usize)
+					.and_then(|author| {
+						schnorrkel::PublicKey::from_bytes(author.0.as_slice()).ok()
+					})
+					.and_then(|pubkey| {
+						let transcript = sp_consensus_babe::make_transcript(
+							&Self::randomness(),
+							current_slot,
+							EpochIndex::get(),
+						);
+
+						primary.vrf_output.0.attach_input_hash(
+							&pubkey,
+							transcript
+						).ok()
+					})
+					.map(|inout| {
+						inout.make_bytes(&sp_consensus_babe::BABE_VRF_INOUT_CONTEXT)
+					})
 			} else {
 				None
 			}
 		});
 
-		Initialized::put(maybe_vrf);
+		Initialized::put(maybe_randomness);
 
 		// enact epoch change, if necessary.
 		T::EpochChangeTrigger::trigger::<T>(now)
@@ -577,7 +599,7 @@ impl<T: Trait> pallet_session::OneSessionHandler<T::AccountId> for Module<T> {
 fn compute_randomness(
 	last_epoch_randomness: schnorrkel::Randomness,
 	epoch_index: u64,
-	rho: impl Iterator<Item=schnorrkel::RawVRFOutput>,
+	rho: impl Iterator<Item=schnorrkel::Randomness>,
 	rho_size_hint: Option<usize>,
 ) -> schnorrkel::Randomness {
 	let mut s = Vec::with_capacity(40 + rho_size_hint.unwrap_or(0) * VRF_OUTPUT_LENGTH);

substrate/frame/babe/src/mock.rs

@@ -30,11 +30,12 @@ use frame_support::{
 	weights::Weight,
 };
 use sp_io;
-use sp_core::H256;
-use sp_consensus_vrf::schnorrkel::{RawVRFOutput, RawVRFProof};
+use sp_core::{H256, U256, crypto::Pair};
+use sp_consensus_babe::AuthorityPair;
+use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof};
 
 impl_outer_origin!{
-	pub enum Origin for Test  where system = frame_system {}
+	pub enum Origin for Test where system = frame_system {}
 }
 
 type DummyValidatorId = u64;
@@ -109,16 +110,20 @@ impl Trait for Test {
 	type EpochChangeTrigger = crate::ExternalTrigger;
 }
 
-pub fn new_test_ext(authorities: Vec<DummyValidatorId>) -> sp_io::TestExternalities {
+pub fn new_test_ext(authorities_len: usize) -> (Vec<AuthorityPair>, sp_io::TestExternalities) {
+	let pairs = (0..authorities_len).map(|i| {
+		AuthorityPair::from_seed(&U256::from(i).into())
+	}).collect::<Vec<_>>();
+
 	let mut t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
 	GenesisConfig {
-		authorities: authorities.into_iter().map(|a| (UintAuthorityId(a).to_public_key(), 1)).collect(),
+		authorities: pairs.iter().map(|a| (a.public(), 1)).collect(),
 	}.assimilate_storage::<Test>(&mut t).unwrap();
-	t.into()
+	(pairs, t.into())
 }
 
 pub fn go_to_block(n: u64, s: u64) {
-	let pre_digest = make_pre_digest(0, s, RawVRFOutput([1; 32]), RawVRFProof([0xff; 64]));
+	let pre_digest = make_secondary_plain_pre_digest(0, s);
 	System::initialize(&n, &Default::default(), &Default::default(), &pre_digest, InitKind::Full);
 	System::set_block_number(n);
 	if s > 1 {
@@ -140,11 +145,11 @@ pub fn progress_to_block(n: u64) {
 pub fn make_pre_digest(
 	authority_index: sp_consensus_babe::AuthorityIndex,
 	slot_number: sp_consensus_babe::SlotNumber,
-	vrf_output: RawVRFOutput,
-	vrf_proof: RawVRFProof,
+	vrf_output: VRFOutput,
+	vrf_proof: VRFProof,
 ) -> Digest {
-	let digest_data = sp_consensus_babe::digests::RawPreDigest::Primary(
-		sp_consensus_babe::digests::RawPrimaryPreDigest {
+	let digest_data = sp_consensus_babe::digests::PreDigest::Primary(
+		sp_consensus_babe::digests::PrimaryPreDigest {
 			authority_index,
 			slot_number,
 			vrf_output,
@@ -155,6 +160,20 @@ pub fn make_pre_digest(
 	Digest { logs: vec![log] }
 }
 
+pub fn make_secondary_plain_pre_digest(
+	authority_index: sp_consensus_babe::AuthorityIndex,
+	slot_number: sp_consensus_babe::SlotNumber,
+) -> Digest {
+	let digest_data = sp_consensus_babe::digests::PreDigest::SecondaryPlain(
+		sp_consensus_babe::digests::SecondaryPlainPreDigest {
+			authority_index,
+			slot_number,
+		}
+	);
+	let log = DigestItem::PreRuntime(sp_consensus_babe::BABE_ENGINE_ID, digest_data.encode());
+	Digest { logs: vec![log] }
+}
+
 pub type System = frame_system::Module<Test>;
 pub type Babe = Module<Test>;
 pub type Session = pallet_session::Module<Test>;

substrate/frame/babe/src/tests.rs

@@ -20,7 +20,8 @@ use super::*;
 use mock::*;
 use frame_support::traits::OnFinalize;
 use pallet_session::ShouldEndSession;
-use sp_consensus_vrf::schnorrkel::{RawVRFOutput, RawVRFProof};
+use sp_core::crypto::IsWrappedBy;
+use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof};
 
 const EMPTY_RANDOMNESS: [u8; 32] = [
 	74, 25, 49, 128, 53, 97, 244, 49,
@@ -37,14 +38,14 @@ fn empty_randomness_is_correct() {
 
 #[test]
 fn initial_values() {
-	new_test_ext(vec![0, 1, 2, 3]).execute_with(|| {
+	new_test_ext(4).1.execute_with(|| {
 		assert_eq!(Babe::authorities().len(), 4)
 	})
 }
 
 #[test]
 fn check_module() {
-	new_test_ext(vec![0, 1, 2, 3]).execute_with(|| {
+	new_test_ext(4).1.execute_with(|| {
 		assert!(!Babe::should_end_session(0), "Genesis does not change sessions");
 		assert!(!Babe::should_end_session(200000),
 			"BABE does not include the block number in epoch calculations");
@@ -53,14 +54,29 @@ fn check_module() {
 
 #[test]
 fn first_block_epoch_zero_start() {
-	new_test_ext(vec![0, 1, 2, 3]).execute_with(|| {
+	let (pairs, mut ext) = new_test_ext(4);
+
+	ext.execute_with(|| {
 		let genesis_slot = 100;
-		let first_vrf = RawVRFOutput([1; 32]);
+
+		let pair = sp_core::sr25519::Pair::from_ref(&pairs[0]).as_ref();
+		let transcript = sp_consensus_babe::make_transcript(
+			&Babe::randomness(),
+			genesis_slot,
+			0,
+		);
+		let vrf_inout = pair.vrf_sign(transcript);
+		let vrf_randomness: sp_consensus_vrf::schnorrkel::Randomness = vrf_inout.0
+			.make_bytes::<[u8; 32]>(&sp_consensus_babe::BABE_VRF_INOUT_CONTEXT);
+		let vrf_output = VRFOutput(vrf_inout.0.to_output());
+		let vrf_proof = VRFProof(vrf_inout.1);
+
+		let first_vrf = vrf_output;
 		let pre_digest = make_pre_digest(
 			0,
 			genesis_slot,
 			first_vrf.clone(),
-			RawVRFProof([0xff; 64]),
+			vrf_proof,
 		);
 
 		assert_eq!(Babe::genesis_slot(), 0);
@@ -83,7 +99,7 @@ fn first_block_epoch_zero_start() {
 		let header = System::finalize();
 
 		assert_eq!(SegmentIndex::get(), 0);
-		assert_eq!(UnderConstruction::get(0), vec![first_vrf]);
+		assert_eq!(UnderConstruction::get(0), vec![vrf_randomness]);
 		assert_eq!(Babe::randomness(), [0; 32]);
 		assert_eq!(NextRandomness::get(), [0; 32]);
 
@@ -91,10 +107,9 @@ fn first_block_epoch_zero_start() {
 		assert_eq!(pre_digest.logs.len(), 1);
 		assert_eq!(header.digest.logs[0], pre_digest.logs[0]);
 
-		let authorities = Babe::authorities();
 		let consensus_log = sp_consensus_babe::ConsensusLog::NextEpochData(
 			sp_consensus_babe::digests::NextEpochDescriptor {
-				authorities,
+				authorities: Babe::authorities(),
 				randomness: Babe::randomness(),
 			}
 		);
@@ -107,7 +122,7 @@ fn first_block_epoch_zero_start() {
 
 #[test]
 fn authority_index() {
-	new_test_ext(vec![0, 1, 2, 3]).execute_with(|| {
+	new_test_ext(4).1.execute_with(|| {
 		assert_eq!(
 			Babe::find_author((&[(BABE_ENGINE_ID, &[][..])]).into_iter().cloned()), None,
 			"Trivially invalid authorities are ignored")
@@ -116,7 +131,7 @@ fn authority_index() {
 
 #[test]
 fn can_predict_next_epoch_change() {
-	new_test_ext(vec![]).execute_with(|| {
+	new_test_ext(0).1.execute_with(|| {
 		assert_eq!(<Test as Trait>::EpochDuration::get(), 3);
 		// this sets the genesis slot to 6;
 		go_to_block(1, 6);