Larger Test Keyring Support (#842)

* Allow creation of authority lists with any number of authorities

* Move keyring helpers into their own module

* Add helper for generating list of test accounts

* Fix import names in tests

* Rename Keyring trait to Signer

* Get list of accounts in a more functional way

* Clarify meaning of `test_keyring` return type

* Use concrete test account type instead of generics

* Make sure voter set contains all authorities which signed off on pre-commits

bridges/modules/grandpa/src/lib.rs

@@ -526,8 +526,8 @@ mod tests {
 	use super::*;
 	use crate::mock::{run_test, test_header, Origin, TestHash, TestHeader, TestNumber, TestRuntime};
 	use bp_test_utils::{
-		authority_list, make_default_justification, make_justification_for_header, JustificationGeneratorParams,
-		Keyring::{Alice, Bob},
+		authority_list, make_default_justification, make_justification_for_header, JustificationGeneratorParams, ALICE,
+		BOB,
 	};
 	use codec::Encode;
 	use frame_support::weights::PostDispatchInfo;
@@ -570,7 +570,7 @@ mod tests {
 
 	fn change_log(delay: u64) -> Digest<TestHash> {
 		let consensus_log = ConsensusLog::<TestNumber>::ScheduledChange(sp_finality_grandpa::ScheduledChange {
-			next_authorities: vec![(Alice.into(), 1), (Bob.into(), 1)],
+			next_authorities: vec![(ALICE.into(), 1), (BOB.into(), 1)],
 			delay,
 		});
 
@@ -583,7 +583,7 @@ mod tests {
 		let consensus_log = ConsensusLog::<TestNumber>::ForcedChange(
 			delay,
 			sp_finality_grandpa::ScheduledChange {
-				next_authorities: vec![(Alice.into(), 1), (Bob.into(), 1)],
+				next_authorities: vec![(ALICE.into(), 1), (BOB.into(), 1)],
 				delay,
 			},
 		);
@@ -760,7 +760,7 @@ mod tests {
 		run_test(|| {
 			let genesis = test_header(0);
 
-			let invalid_authority_list = vec![(Alice.into(), u64::MAX), (Bob.into(), 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,
@@ -797,7 +797,7 @@ mod tests {
 			initialize_substrate_bridge();
 
 			let next_set_id = 2;
-			let next_authorities = vec![(Alice.into(), 1), (Bob.into(), 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.

bridges/modules/substrate/src/fork_tests.rs

@@ -59,10 +59,7 @@ use crate::storage::ImportedHeader;
 use crate::verifier::*;
 use crate::{BestFinalized, BestHeight, BridgeStorage, NextScheduledChange, PalletStorage};
 use bp_header_chain::AuthoritySet;
-use bp_test_utils::{
-	authority_list, make_default_justification,
-	Keyring::{Alice, Bob},
-};
+use bp_test_utils::{authority_list, make_default_justification, ALICE, BOB};
 use codec::Encode;
 use frame_support::{IterableStorageMap, StorageValue};
 use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID};
@@ -505,7 +502,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.into(), 1), (Bob.into(), 1)],
+		next_authorities: vec![(ALICE.into(), 1), (BOB.into(), 1)],
 		delay,
 	});
 

bridges/modules/substrate/src/lib.rs

@@ -721,10 +721,7 @@ 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::{
-		authority_list,
-		Keyring::{Alice, Bob},
-	};
+	use bp_test_utils::{authority_list, ALICE, BOB};
 	use frame_support::{assert_err, assert_noop, assert_ok};
 	use sp_runtime::DispatchError;
 
@@ -952,7 +949,7 @@ mod tests {
 			let storage = PalletStorage::<TestRuntime>::new();
 
 			let next_set_id = 2;
-			let next_authorities = vec![(Alice.into(), 1), (Bob.into(), 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.
@@ -981,7 +978,7 @@ mod tests {
 			let storage = PalletStorage::<TestRuntime>::new();
 
 			let next_set_id = 2;
-			let next_authorities = vec![(Alice.into(), 1), (Bob.into(), 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.
@@ -1010,7 +1007,7 @@ mod tests {
 		run_test(|| {
 			let storage = PalletStorage::<TestRuntime>::new();
 
-			let next_authorities = vec![(Alice.into(), 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,10 +342,7 @@ mod tests {
 	use super::*;
 	use crate::mock::*;
 	use crate::{BestFinalized, BestHeight, HeaderId, ImportedHeaders, PalletStorage};
-	use bp_test_utils::{
-		authority_list, make_default_justification,
-		Keyring::{Alice, Bob},
-	};
+	use bp_test_utils::{authority_list, make_default_justification, ALICE, BOB};
 	use codec::Encode;
 	use frame_support::{assert_err, assert_ok};
 	use frame_support::{StorageMap, StorageValue};
@@ -695,7 +692,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.into(), u64::MAX), (Bob.into(), u64::MAX)],
+				next_authorities: vec![(ALICE.into(), u64::MAX), (BOB.into(), u64::MAX)],
 				delay: 0,
 			});
 
@@ -800,7 +797,7 @@ mod tests {
 			// Schedule a change at the height of our header
 			let set_id = 2;
 			let height = *header.number();
-			let authorities = vec![Alice.into()];
+			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

@@ -17,7 +17,6 @@
 //! Tests for Grandpa Justification code.
 
 use bp_header_chain::justification::{verify_justification, Error};
-use bp_test_utils::Keyring::*;
 use bp_test_utils::*;
 use codec::Encode;
 
@@ -29,7 +28,7 @@ fn valid_justification_accepted() {
 		header: test_header(1),
 		round: TEST_GRANDPA_ROUND,
 		set_id: TEST_GRANDPA_SET_ID,
-		authorities: vec![(Alice, 1), (Bob, 1), (Charlie, 1), (Dave, 1), (Eve, 1)],
+		authorities: vec![(ALICE, 1), (BOB, 1), (CHARLIE, 1), (DAVE, 1), (EVE, 1)],
 		depth: 5,
 		forks: 5,
 	};
@@ -51,7 +50,7 @@ fn valid_justification_accepted_with_single_fork() {
 		header: test_header(1),
 		round: TEST_GRANDPA_ROUND,
 		set_id: TEST_GRANDPA_SET_ID,
-		authorities: vec![(Alice, 1), (Bob, 1), (Charlie, 1), (Dave, 1), (Eve, 1)],
+		authorities: vec![(ALICE, 1), (BOB, 1), (CHARLIE, 1), (DAVE, 1), (EVE, 1)],
 		depth: 5,
 		forks: 1,
 	};
@@ -67,6 +66,40 @@ fn valid_justification_accepted_with_single_fork() {
 	);
 }
 
+#[test]
+fn valid_justification_accepted_with_arbitrary_number_of_authorities() {
+	use finality_grandpa::voter_set::VoterSet;
+	use sp_finality_grandpa::AuthorityId;
+
+	let n = 15;
+	let authorities = accounts(n).iter().map(|k| (*k, 1)).collect::<Vec<_>>();
+
+	let params = JustificationGeneratorParams {
+		header: test_header(1),
+		round: TEST_GRANDPA_ROUND,
+		set_id: TEST_GRANDPA_SET_ID,
+		authorities: authorities.clone(),
+		depth: 5,
+		forks: n.into(),
+	};
+
+	let authorities = authorities
+		.iter()
+		.map(|(id, w)| (AuthorityId::from(*id), *w))
+		.collect::<Vec<(AuthorityId, _)>>();
+	let voter_set = VoterSet::new(authorities).unwrap();
+
+	assert_eq!(
+		verify_justification::<TestHeader>(
+			header_id::<TestHeader>(1),
+			TEST_GRANDPA_SET_ID,
+			&voter_set,
+			&make_justification_for_header::<TestHeader>(params).encode()
+		),
+		Ok(()),
+	);
+}
+
 #[test]
 fn justification_with_invalid_encoding_rejected() {
 	assert_eq!(
@@ -143,7 +176,7 @@ fn justification_is_invalid_if_we_dont_meet_threshold() {
 		header: test_header(1),
 		round: TEST_GRANDPA_ROUND,
 		set_id: TEST_GRANDPA_SET_ID,
-		authorities: vec![(Alice, 1), (Bob, 1)],
+		authorities: vec![(ALICE, 1), (BOB, 1)],
 		depth: 2,
 		forks: 2,
 	};

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

@@ -0,0 +1,91 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Parity Bridges Common.
+
+// Parity Bridges Common is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Bridges Common is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Utilities for working with test accounts.
+
+use ed25519_dalek::{Keypair, PublicKey, SecretKey, Signature};
+use finality_grandpa::voter_set::VoterSet;
+use sp_application_crypto::Public;
+use sp_finality_grandpa::{AuthorityId, AuthorityList, AuthorityWeight};
+use sp_runtime::RuntimeDebug;
+
+/// Set of test accounts with friendly names.
+pub const ALICE: Account = Account(0);
+pub const BOB: Account = Account(1);
+pub const CHARLIE: Account = Account(2);
+pub const DAVE: Account = Account(3);
+pub const EVE: Account = Account(4);
+pub const FERDIE: Account = Account(5);
+
+/// A test account which can be used to sign messages.
+#[derive(RuntimeDebug, Clone, Copy)]
+pub struct Account(pub u8);
+
+impl Account {
+	pub fn public(&self) -> PublicKey {
+		(&self.secret()).into()
+	}
+
+	pub fn secret(&self) -> SecretKey {
+		SecretKey::from_bytes(&[self.0; 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 {
+		use ed25519_dalek::Signer;
+		self.pair().sign(msg)
+	}
+}
+
+impl From<Account> for AuthorityId {
+	fn from(p: Account) -> Self {
+		AuthorityId::from_slice(&p.public().to_bytes())
+	}
+}
+
+/// Get a valid set of voters for a Grandpa round.
+pub fn voter_set() -> VoterSet<AuthorityId> {
+	VoterSet::new(authority_list()).unwrap()
+}
+
+/// Convenience function to get a list of Grandpa authorities.
+pub fn authority_list() -> AuthorityList {
+	test_keyring()
+		.iter()
+		.map(|(id, w)| (AuthorityId::from(*id), *w))
+		.collect()
+}
+
+/// Get the corresponding identities from the keyring for the "standard" authority set.
+pub fn test_keyring() -> Vec<(Account, AuthorityWeight)> {
+	vec![(ALICE, 1), (BOB, 1), (CHARLIE, 1)]
+}
+
+/// Get a list of "unique" accounts.
+pub fn accounts(len: u8) -> Vec<Account> {
+	(0..len).into_iter().map(Account).collect()
+}

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

@@ -19,15 +19,17 @@
 #![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_application_crypto::TryFrom;
+use sp_finality_grandpa::{AuthorityId, AuthorityWeight};
 use sp_finality_grandpa::{AuthoritySignature, SetId};
 use sp_runtime::traits::{Header as HeaderT, One, Zero};
-use sp_runtime::RuntimeDebug;
 use sp_std::prelude::*;
 
+// Re-export all our test account utilities
+pub use keyring::*;
+
+mod keyring;
+
 pub const TEST_GRANDPA_ROUND: u64 = 1;
 pub const TEST_GRANDPA_SET_ID: SetId = 1;
 
@@ -40,7 +42,7 @@ pub struct JustificationGeneratorParams<H> {
 	/// The current authority set ID.
 	pub set_id: SetId,
 	/// The current GRANDPA authority set.
-	pub authorities: Vec<(Keyring, AuthorityWeight)>,
+	pub authorities: Vec<(Account, AuthorityWeight)>,
 	/// The number of headers included in our justification's vote ancestries.
 	pub depth: u32,
 	/// The number of forks, and thus the number of pre-commits in our justification.
@@ -53,7 +55,7 @@ impl<H: HeaderT> Default for JustificationGeneratorParams<H> {
 			header: test_header(One::one()),
 			round: TEST_GRANDPA_ROUND,
 			set_id: TEST_GRANDPA_SET_ID,
-			authorities: keyring(),
+			authorities: test_keyring(),
 			depth: 2,
 			forks: 1,
 		}
@@ -157,7 +159,7 @@ fn generate_chain<H: HeaderT>(fork_id: u8, depth: u32, ancestor: &H) -> Vec<H> {
 }
 
 fn signed_precommit<H: HeaderT>(
-	signer: &Keyring,
+	signer: &Account,
 	target: (H::Hash, H::Number),
 	round: u64,
 	set_id: SetId,
@@ -170,7 +172,7 @@ fn signed_precommit<H: HeaderT>(
 	let encoded =
 		sp_finality_grandpa::localized_payload(round, set_id, &finality_grandpa::Message::Precommit(precommit.clone()));
 
-	let signature = signer.pair().sign(&encoded);
+	let signature = signer.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
@@ -211,61 +213,3 @@ pub fn test_header<H: HeaderT>(number: H::Number) -> H {
 pub fn header_id<H: HeaderT>(index: u8) -> (H::Hash, H::Number) {
 	(test_header::<H>(index.into()).hash(), index.into())
 }
-
-/// 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.
-pub fn voter_set() -> VoterSet<AuthorityId> {
-	VoterSet::new(authority_list()).unwrap()
-}
-
-/// Convenience function to get a list of Grandpa authorities.
-pub fn authority_list() -> AuthorityList {
-	keyring().iter().map(|(id, w)| (AuthorityId::from(*id), *w)).collect()
-}
-
-/// 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)]
-}