From 78a9cdca66d38c2ee870e01166fb4eb475e8a8c7 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 24 Mar 2021 16:14:15 -0400 Subject: [PATCH] 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 | 12 +-- bridges/modules/substrate/src/fork_tests.rs | 7 +- bridges/modules/substrate/src/lib.rs | 11 +-- bridges/modules/substrate/src/verifier.rs | 9 +- .../header-chain/tests/justification.rs | 41 ++++++++- bridges/primitives/test-utils/src/keyring.rs | 91 +++++++++++++++++++ bridges/primitives/test-utils/src/lib.rs | 78 +++------------- 7 files changed, 154 insertions(+), 95 deletions(-) create mode 100644 bridges/primitives/test-utils/src/keyring.rs diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 21ec2d6528..4f3afcbbf7 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/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 { let consensus_log = ConsensusLog::::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::::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. diff --git a/bridges/modules/substrate/src/fork_tests.rs b/bridges/modules/substrate/src/fork_tests.rs index acd147109a..40b40678a8 100644 --- a/bridges/modules/substrate/src/fork_tests.rs +++ b/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 { let consensus_log = ConsensusLog::::ScheduledChange(sp_finality_grandpa::ScheduledChange { - next_authorities: vec![(Alice.into(), 1), (Bob.into(), 1)], + next_authorities: vec![(ALICE.into(), 1), (BOB.into(), 1)], delay, }); diff --git a/bridges/modules/substrate/src/lib.rs b/bridges/modules/substrate/src/lib.rs index 8a53ed336e..ce40066885 100644 --- a/bridges/modules/substrate/src/lib.rs +++ b/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::::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::::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::::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); diff --git a/bridges/modules/substrate/src/verifier.rs b/bridges/modules/substrate/src/verifier.rs index 4849941124..f2e33e21e4 100644 --- a/bridges/modules/substrate/src/verifier.rs +++ b/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::::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()); diff --git a/bridges/primitives/header-chain/tests/justification.rs b/bridges/primitives/header-chain/tests/justification.rs index 29ba744133..33808461f6 100644 --- a/bridges/primitives/header-chain/tests/justification.rs +++ b/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::>(); + + 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::>(); + let voter_set = VoterSet::new(authorities).unwrap(); + + assert_eq!( + verify_justification::( + header_id::(1), + TEST_GRANDPA_SET_ID, + &voter_set, + &make_justification_for_header::(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, }; diff --git a/bridges/primitives/test-utils/src/keyring.rs b/bridges/primitives/test-utils/src/keyring.rs new file mode 100644 index 0000000000..5b96b98a3a --- /dev/null +++ b/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 . + +//! 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 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 { + 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 { + (0..len).into_iter().map(Account).collect() +} diff --git a/bridges/primitives/test-utils/src/lib.rs b/bridges/primitives/test-utils/src/lib.rs index 31dabde5ea..f6c2f6d094 100644 --- a/bridges/primitives/test-utils/src/lib.rs +++ b/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 { /// 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 Default for JustificationGeneratorParams { 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(fork_id: u8, depth: u32, ancestor: &H) -> Vec { } fn signed_precommit( - signer: &Keyring, + signer: &Account, target: (H::Hash, H::Number), round: u64, set_id: SetId, @@ -170,7 +172,7 @@ fn signed_precommit( 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 = 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(number: H::Number) -> H { pub fn header_id(index: u8) -> (H::Hash, H::Number) { (test_header::(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 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 { - 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)] -}