Make Test Helpers no_std Compatible (#823)

* Use `sp-application-crypto` when generating test justifications

* Try to justifications tests compiling

* Try using `ed25519_dalek` for justification creation

* Small cleanup

* Correctly generate a Keypair

The public key didn't correspond to what would be derived
with the given private key.

* Remove enum index hack

* Clean up test account helper functions

* Fix tests in `pallet-finality-verifier`

* Get `pallet-substrate-bridge` tests compiling again

* Use the correct keyring in justification tests

* Clean up Keyring related code a bit

* Appease Clippy

* Remove unused import

* Use keyring variants directly

* Remove unecessary From implementation

bridges/modules/finality-verifier/src/lib.rs

@@ -521,7 +521,10 @@ pub fn initialize_for_benchmarks<T: Config<I>, I: 'static>(header: BridgedHeader
 mod tests {
 	use super::*;
 	use crate::mock::{run_test, test_header, Origin, TestHash, TestHeader, TestNumber, TestRuntime};
-	use bp_test_utils::{alice, authority_list, bob, make_justification_for_header};
+	use bp_test_utils::{
+		authority_list, keyring, make_justification_for_header,
+		Keyring::{Alice, Bob},
+	};
 	use codec::Encode;
 	use frame_support::weights::PostDispatchInfo;
 	use frame_support::{assert_err, assert_noop, assert_ok};
@@ -551,7 +554,7 @@ mod tests {
 
 		let set_id = 1;
 		let grandpa_round = 1;
-		let justification = make_justification_for_header(&header, grandpa_round, set_id, &authority_list()).encode();
+		let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 		Module::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification)
 	}
@@ -566,7 +569,7 @@ mod tests {
 
 	fn change_log(delay: u64) -> Digest<TestHash> {
 		let consensus_log = ConsensusLog::<TestNumber>::ScheduledChange(sp_finality_grandpa::ScheduledChange {
-			next_authorities: vec![(alice(), 1), (bob(), 1)],
+			next_authorities: vec![(Alice.into(), 1), (Bob.into(), 1)],
 			delay,
 		});
 
@@ -579,7 +582,7 @@ mod tests {
 		let consensus_log = ConsensusLog::<TestNumber>::ForcedChange(
 			delay,
 			sp_finality_grandpa::ScheduledChange {
-				next_authorities: vec![(alice(), 1), (bob(), 1)],
+				next_authorities: vec![(Alice.into(), 1), (Bob.into(), 1)],
 				delay,
 			},
 		);
@@ -725,8 +728,7 @@ mod tests {
 
 			let set_id = 2;
 			let grandpa_round = 1;
-			let justification =
-				make_justification_for_header(&header, grandpa_round, set_id, &authority_list()).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			assert_err!(
 				Module::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification,),
@@ -753,11 +755,9 @@ mod tests {
 	#[test]
 	fn disallows_invalid_authority_set() {
 		run_test(|| {
-			use bp_test_utils::{alice, bob};
-
 			let genesis = test_header(0);
 
-			let invalid_authority_list = vec![(alice(), u64::MAX), (bob(), u64::MAX)];
+			let invalid_authority_list = vec![(Alice.into(), u64::MAX), (Bob.into(), u64::MAX)];
 			let init_data = InitializationData {
 				header: genesis,
 				authority_list: invalid_authority_list,
@@ -794,7 +794,7 @@ mod tests {
 			initialize_substrate_bridge();
 
 			let next_set_id = 2;
-			let next_authorities = vec![(alice(), 1), (bob(), 1)];
+			let next_authorities = vec![(Alice.into(), 1), (Bob.into(), 1)];
 
 			// Need to update the header digest to indicate that our header signals an authority set
 			// change. The change will be enacted when we import our header.
@@ -804,8 +804,7 @@ mod tests {
 			// Create a valid justification for the header
 			let set_id = 1;
 			let grandpa_round = 1;
-			let justification =
-				make_justification_for_header(&header, grandpa_round, set_id, &authority_list()).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			// Let's import our test header
 			assert_ok!(Module::<TestRuntime>::submit_finality_proof(
@@ -839,8 +838,7 @@ mod tests {
 			// Create a valid justification for the header
 			let set_id = 1;
 			let grandpa_round = 1;
-			let justification =
-				make_justification_for_header(&header, grandpa_round, set_id, &authority_list()).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			// Should not be allowed to import this header
 			assert_err!(
@@ -863,8 +861,7 @@ mod tests {
 			// Create a valid justification for the header
 			let set_id = 1;
 			let grandpa_round = 1;
-			let justification =
-				make_justification_for_header(&header, grandpa_round, set_id, &authority_list()).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			// Should not be allowed to import this header
 			assert_err!(

bridges/modules/substrate/src/fork_tests.rs

@@ -59,7 +59,10 @@ use crate::storage::ImportedHeader;
 use crate::verifier::*;
 use crate::{BestFinalized, BestHeight, BridgeStorage, NextScheduledChange, PalletStorage};
 use bp_header_chain::AuthoritySet;
-use bp_test_utils::{alice, authority_list, bob, make_justification_for_header};
+use bp_test_utils::{
+	authority_list, keyring, make_justification_for_header,
+	Keyring::{Alice, Bob},
+};
 use codec::Encode;
 use frame_support::{IterableStorageMap, StorageValue};
 use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID};
@@ -456,8 +459,7 @@ where
 				// See for more: https://github.com/paritytech/parity-bridges-common/issues/430
 				let grandpa_round = 1;
 				let set_id = 1;
-				let authorities = authority_list();
-				let justification = make_justification_for_header(header, grandpa_round, set_id, &authorities).encode();
+				let justification = make_justification_for_header(header, grandpa_round, set_id, &keyring()).encode();
 
 				let res = verifier
 					.import_finality_proof(header.hash(), justification.into())
@@ -505,7 +507,7 @@ where
 
 pub(crate) fn change_log(delay: u64) -> Digest<TestHash> {
 	let consensus_log = ConsensusLog::<TestNumber>::ScheduledChange(sp_finality_grandpa::ScheduledChange {
-		next_authorities: vec![(alice(), 1), (bob(), 1)],
+		next_authorities: vec![(Alice.into(), 1), (Bob.into(), 1)],
 		delay,
 	});
 

bridges/modules/substrate/src/lib.rs

@@ -721,7 +721,10 @@ mod tests {
 	use super::*;
 	use crate::mock::{run_test, test_header, unfinalized_header, Origin, TestHeader, TestRuntime};
 	use bp_header_chain::HeaderChain;
-	use bp_test_utils::{alice, authority_list, bob};
+	use bp_test_utils::{
+		authority_list,
+		Keyring::{Alice, Bob},
+	};
 	use frame_support::{assert_err, assert_noop, assert_ok};
 	use sp_runtime::DispatchError;
 
@@ -949,7 +952,7 @@ mod tests {
 			let storage = PalletStorage::<TestRuntime>::new();
 
 			let next_set_id = 2;
-			let next_authorities = vec![(alice(), 1), (bob(), 1)];
+			let next_authorities = vec![(Alice.into(), 1), (Bob.into(), 1)];
 
 			// Need to update the header digest to indicate that our header signals an authority set
 			// change. The change will be enacted when we import our header.
@@ -978,7 +981,7 @@ mod tests {
 			let storage = PalletStorage::<TestRuntime>::new();
 
 			let next_set_id = 2;
-			let next_authorities = vec![(alice(), 1), (bob(), 1)];
+			let next_authorities = vec![(Alice.into(), 1), (Bob.into(), 1)];
 
 			// Need to update the header digest to indicate that our header signals an authority set
 			// change. However, the change doesn't happen until the next block.
@@ -1007,7 +1010,7 @@ mod tests {
 		run_test(|| {
 			let storage = PalletStorage::<TestRuntime>::new();
 
-			let next_authorities = vec![(alice(), 1)];
+			let next_authorities = vec![(Alice.into(), 1)];
 			let next_set_id = 2;
 			let next_authority_set = AuthoritySet::new(next_authorities.clone(), next_set_id);
 

bridges/modules/substrate/src/verifier.rs

@@ -342,7 +342,10 @@ mod tests {
 	use super::*;
 	use crate::mock::*;
 	use crate::{BestFinalized, BestHeight, HeaderId, ImportedHeaders, PalletStorage};
-	use bp_test_utils::{alice, authority_list, bob, make_justification_for_header};
+	use bp_test_utils::{
+		authority_list, keyring, make_justification_for_header,
+		Keyring::{Alice, Bob},
+	};
 	use codec::Encode;
 	use frame_support::{assert_err, assert_ok};
 	use frame_support::{StorageMap, StorageValue};
@@ -587,7 +590,7 @@ mod tests {
 
 			let set_id = 1;
 			let authorities = authority_list();
-			let initial_authority_set = AuthoritySet::new(authorities.clone(), set_id);
+			let initial_authority_set = AuthoritySet::new(authorities, set_id);
 			storage.update_current_authority_set(initial_authority_set);
 
 			// Let's import our header
@@ -603,7 +606,7 @@ mod tests {
 
 			// Now lets finalize our best header
 			let grandpa_round = 1;
-			let justification = make_justification_for_header(&header, grandpa_round, set_id, &authorities).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 			assert_ok!(verifier.import_finality_proof(header.hash(), justification.into()));
 
 			// Our best header should only appear once in the list of best headers
@@ -693,7 +696,7 @@ mod tests {
 			// This is an *invalid* authority set because the combined weight of the
 			// authorities is greater than `u64::MAX`
 			let consensus_log = ConsensusLog::<TestNumber>::ScheduledChange(sp_finality_grandpa::ScheduledChange {
-				next_authorities: vec![(alice(), u64::MAX), (bob(), u64::MAX)],
+				next_authorities: vec![(Alice.into(), u64::MAX), (Bob.into(), u64::MAX)],
 				delay: 0,
 			});
 
@@ -722,12 +725,12 @@ mod tests {
 
 			let set_id = 1;
 			let authorities = authority_list();
-			let authority_set = AuthoritySet::new(authorities.clone(), set_id);
+			let authority_set = AuthoritySet::new(authorities, set_id);
 			storage.update_current_authority_set(authority_set);
 
 			// We'll need this justification to finalize the header
 			let grandpa_round = 1;
-			let justification = make_justification_for_header(&header, grandpa_round, set_id, &authorities).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			let mut verifier = Verifier {
 				storage: storage.clone(),
@@ -748,14 +751,11 @@ mod tests {
 
 			let set_id = 1;
 			let authorities = authority_list();
-			let authority_set = AuthoritySet {
-				authorities: authorities.clone(),
-				set_id,
-			};
+			let authority_set = AuthoritySet { authorities, set_id };
 			storage.update_current_authority_set(authority_set);
 
 			let grandpa_round = 1;
-			let justification = make_justification_for_header(&header, grandpa_round, set_id, &authorities).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			let mut verifier = Verifier {
 				storage: storage.clone(),
@@ -792,19 +792,19 @@ mod tests {
 
 			let set_id = 1;
 			let authorities = authority_list();
-			let initial_authority_set = AuthoritySet::new(authorities.clone(), set_id);
+			let initial_authority_set = AuthoritySet::new(authorities, set_id);
 			storage.update_current_authority_set(initial_authority_set);
 
 			// This header enacts an authority set change upon finalization
 			let header = test_header(2);
 
 			let grandpa_round = 1;
-			let justification = make_justification_for_header(&header, grandpa_round, set_id, &authorities).encode();
+			let justification = make_justification_for_header(&header, grandpa_round, set_id, &keyring()).encode();
 
 			// Schedule a change at the height of our header
 			let set_id = 2;
 			let height = *header.number();
-			let authorities = vec![alice()];
+			let authorities = vec![Alice.into()];
 			let change = schedule_next_change(authorities, set_id, height);
 			storage.schedule_next_set_change(genesis_hash, change.clone());
 

bridges/primitives/header-chain/tests/justification.rs

@@ -23,12 +23,7 @@ use codec::Encode;
 type TestHeader = sp_runtime::testing::Header;
 
 fn make_justification_for_header_1() -> GrandpaJustification<TestHeader> {
-	make_justification_for_header(
-		&test_header(1),
-		TEST_GRANDPA_ROUND,
-		TEST_GRANDPA_SET_ID,
-		&authority_list(),
-	)
+	make_justification_for_header(&test_header(1), TEST_GRANDPA_ROUND, TEST_GRANDPA_SET_ID, &keyring())
 }
 
 #[test]

bridges/primitives/test-utils/Cargo.toml

@@ -6,8 +6,22 @@ edition = "2018"
 license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 
 [dependencies]
-finality-grandpa = { version = "0.14.0" }
-bp-header-chain = { path = "../header-chain" }
-sp-finality-grandpa = { git = "https://github.com/paritytech/substrate", branch = "master" }
-sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
-sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master" }
+bp-header-chain = { path = "../header-chain", default-features = false  }
+ed25519-dalek = { version = "1.0", default-features = false, features = ["u64_backend"] }
+finality-grandpa = { version = "0.14.0", default-features = false }
+sp-finality-grandpa = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false  }
+sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
+sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
+sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
+
+[features]
+default = ["std"]
+std = [
+	"bp-header-chain/std",
+	"ed25519-dalek/std",
+	"finality-grandpa/std",
+	"sp-finality-grandpa/std",
+	"sp-runtime/std",
+	"sp-std/std",
+	"sp-application-crypto/std",
+]

bridges/primitives/test-utils/src/lib.rs

@@ -15,18 +15,18 @@
 // along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Utilities for testing runtime code.
-//!
-//! Unlike other crates in the `primitives` folder, this crate does *not* need to compile in a
-//! `no_std` environment. This is fine because this code should only be used, as the name implies,
-//! in tests.
+
+#![cfg_attr(not(feature = "std"), no_std)]
 
 use bp_header_chain::justification::GrandpaJustification;
+use ed25519_dalek::{Keypair, PublicKey, SecretKey, Signature, Signer};
 use finality_grandpa::voter_set::VoterSet;
+use sp_application_crypto::{Public, TryFrom};
 use sp_finality_grandpa::{AuthorityId, AuthorityList, AuthorityWeight};
 use sp_finality_grandpa::{AuthoritySignature, SetId};
-use sp_keyring::Ed25519Keyring;
-use sp_runtime::traits::Header as HeaderT;
-use sp_runtime::traits::{One, Zero};
+use sp_runtime::traits::{Header as HeaderT, One, Zero};
+use sp_runtime::RuntimeDebug;
+use sp_std::prelude::*;
 
 pub const TEST_GRANDPA_ROUND: u64 = 1;
 pub const TEST_GRANDPA_SET_ID: SetId = 1;
@@ -37,7 +37,7 @@ pub fn make_justification_for_header<H: HeaderT>(
 	header: &H,
 	round: u64,
 	set_id: SetId,
-	authorities: &[(AuthorityId, AuthorityWeight)],
+	authorities: &[(Keyring, AuthorityWeight)],
 ) -> GrandpaJustification<H> {
 	let (target_hash, target_number) = (header.hash(), *header.number());
 	let mut precommits = vec![];
@@ -51,13 +51,7 @@ pub fn make_justification_for_header<H: HeaderT>(
 	// I'm using the same header for all the voters since it doesn't matter as long
 	// as they all vote on blocks _ahead_ of the one we're interested in finalizing
 	for (id, _weight) in authorities.iter() {
-		let signer = extract_keyring(&id);
-		let precommit = signed_precommit::<H>(
-			signer,
-			(precommit_header.hash(), *precommit_header.number()),
-			round,
-			set_id,
-		);
+		let precommit = signed_precommit::<H>(id, (precommit_header.hash(), *precommit_header.number()), round, set_id);
 		precommits.push(precommit);
 		votes_ancestries.push(precommit_header.clone());
 	}
@@ -74,7 +68,7 @@ pub fn make_justification_for_header<H: HeaderT>(
 }
 
 fn signed_precommit<H: HeaderT>(
-	signer: Ed25519Keyring,
+	signer: &Keyring,
 	target: (H::Hash, H::Number),
 	round: u64,
 	set_id: SetId,
@@ -83,13 +77,24 @@ fn signed_precommit<H: HeaderT>(
 		target_hash: target.0,
 		target_number: target.1,
 	};
+
 	let encoded =
 		sp_finality_grandpa::localized_payload(round, set_id, &finality_grandpa::Message::Precommit(precommit.clone()));
-	let signature = signer.sign(&encoded[..]).into();
+
+	let signature = signer.pair().sign(&encoded);
+	let raw_signature: Vec<u8> = signature.to_bytes().into();
+
+	// Need to wrap our signature and id types that they match what our `SignedPrecommit` is expecting
+	let signature = AuthoritySignature::try_from(raw_signature).expect(
+		"We know our Keypair is good,
+		so our signature must also be good.",
+	);
+	let id = (*signer).into();
+
 	finality_grandpa::SignedPrecommit {
 		precommit,
 		signature,
-		id: signer.public().into(),
+		id,
 	}
 }
 
@@ -118,11 +123,47 @@ pub fn header_id<H: HeaderT>(index: u8) -> (H::Hash, H::Number) {
 	(test_header::<H>(index.into()).hash(), index.into())
 }
 
-/// Get the identity of a test account given an ED25519 Public key.
-pub fn extract_keyring(id: &AuthorityId) -> Ed25519Keyring {
-	let mut raw_public = [0; 32];
-	raw_public.copy_from_slice(id.as_ref());
-	Ed25519Keyring::from_raw_public(raw_public).unwrap()
+/// Set of test accounts.
+#[derive(RuntimeDebug, Clone, Copy)]
+pub enum Keyring {
+	Alice,
+	Bob,
+	Charlie,
+	Dave,
+	Eve,
+	Ferdie,
+}
+
+impl Keyring {
+	pub fn public(&self) -> PublicKey {
+		(&self.secret()).into()
+	}
+
+	pub fn secret(&self) -> SecretKey {
+		SecretKey::from_bytes(&[*self as u8; 32]).expect("A static array of the correct length is a known good.")
+	}
+
+	pub fn pair(self) -> Keypair {
+		let mut pair: [u8; 64] = [0; 64];
+
+		let secret = self.secret();
+		pair[..32].copy_from_slice(&secret.to_bytes());
+
+		let public = self.public();
+		pair[32..].copy_from_slice(&public.to_bytes());
+
+		Keypair::from_bytes(&pair).expect("We expect the SecretKey to be good, so this must also be good.")
+	}
+
+	pub fn sign(self, msg: &[u8]) -> Signature {
+		self.pair().sign(msg)
+	}
+}
+
+impl From<Keyring> for AuthorityId {
+	fn from(k: Keyring) -> Self {
+		AuthorityId::from_slice(&k.public().to_bytes())
+	}
 }
 
 /// Get a valid set of voters for a Grandpa round.
@@ -132,20 +173,10 @@ pub fn voter_set() -> VoterSet<AuthorityId> {
 
 /// Convenience function to get a list of Grandpa authorities.
 pub fn authority_list() -> AuthorityList {
-	vec![(alice(), 1), (bob(), 1), (charlie(), 1)]
+	keyring().iter().map(|(id, w)| (AuthorityId::from(*id), *w)).collect()
 }
 
-/// Get the Public key of the Alice test account.
-pub fn alice() -> AuthorityId {
-	Ed25519Keyring::Alice.public().into()
-}
-
-/// Get the Public key of the Bob test account.
-pub fn bob() -> AuthorityId {
-	Ed25519Keyring::Bob.public().into()
-}
-
-/// Get the Public key of the Charlie test account.
-pub fn charlie() -> AuthorityId {
-	Ed25519Keyring::Charlie.public().into()
+/// Get the corresponding identities from the keyring for the "standard" authority set.
+pub fn keyring() -> Vec<(Keyring, u64)> {
+	vec![(Keyring::Alice, 1), (Keyring::Bob, 1), (Keyring::Charlie, 1)]
 }