diff --git a/Cargo.lock b/Cargo.lock index 2e0f788707dfc..2b946665639be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6269,6 +6269,7 @@ dependencies = [ "sc-telemetry", "serde_json", "sp-api", + "sp-application-crypto", "sp-arithmetic", "sp-blockchain", "sp-consensus", diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs index 17606e29239bf..b9bc68ce3a8b6 100644 --- a/bin/node-template/node/src/service.rs +++ b/bin/node-template/node/src/service.rs @@ -148,7 +148,7 @@ pub fn new_full(config: Configuration) -> Result> NetworkBridge { /// network all within the current set. pub(crate) fn round_communication( &self, + keystore: Option, round: Round, set_id: SetId, voters: Arc>, - local_key: Option, + local_key: Option, has_voted: HasVoted, ) -> ( impl Stream> + Unpin, @@ -287,10 +288,9 @@ impl> NetworkBridge { &*voters, ); - let locals = local_key.and_then(|pair| { - let id = pair.public(); + let local_id = local_key.and_then(|id| { if voters.contains(&id) { - Some((pair, id)) + Some(id) } else { None } @@ -350,10 +350,11 @@ impl> NetworkBridge { let (tx, out_rx) = mpsc::channel(0); let outgoing = OutgoingMessages:: { + keystore: keystore.clone(), round: round.0, set_id: set_id.0, network: self.gossip_engine.clone(), - locals, + local_id, sender: tx, has_voted, }; @@ -628,10 +629,11 @@ pub struct SetId(pub SetIdNumber); pub(crate) struct OutgoingMessages { round: RoundNumber, set_id: SetIdNumber, - locals: Option<(AuthorityPair, AuthorityId)>, + local_id: Option, sender: mpsc::Sender>, network: Arc>>, has_voted: HasVoted, + keystore: Option, } impl Unpin for OutgoingMessages {} @@ -665,14 +667,26 @@ impl Sink> for OutgoingMessages } // when locals exist, sign messages on import - if let Some((ref pair, _)) = self.locals { + if let Some(ref public) = self.local_id { + let keystore = match &self.keystore { + Some(keystore) => keystore.clone(), + None => { + return Err(Error::Signing("Cannot sign without a keystore".to_string())) + } + }; + let target_hash = *(msg.target().0); let signed = sp_finality_grandpa::sign_message( + keystore, msg, - pair, + public.clone(), self.round, self.set_id, - ); + ).ok_or( + Error::Signing(format!( + "Failed to sign GRANDPA vote for round {} targetting {:?}", self.round, target_hash + )) + )?; let message = GossipMessage::Vote(VoteMessage:: { message: signed.clone(), diff --git a/client/finality-grandpa/src/environment.rs b/client/finality-grandpa/src/environment.rs index afcc3891ac39f..6db854bacc13b 100644 --- a/client/finality-grandpa/src/environment.rs +++ b/client/finality-grandpa/src/environment.rs @@ -35,7 +35,6 @@ use finality_grandpa::{ voter, voter_set::VoterSet, }; use sp_blockchain::{HeaderBackend, HeaderMetadata, Error as ClientError}; -use sp_core::Pair; use sp_runtime::generic::BlockId; use sp_runtime::traits::{ Block as BlockT, Header as HeaderT, NumberFor, One, Zero, @@ -704,11 +703,11 @@ where let prevote_timer = Delay::new(self.config.gossip_duration * 2); let precommit_timer = Delay::new(self.config.gossip_duration * 4); - let local_key = crate::is_voter(&self.voters, &self.config.keystore); + let local_key = crate::is_voter(&self.voters, self.config.keystore.as_ref()); let has_voted = match self.voter_set_state.has_voted(round) { HasVoted::Yes(id, vote) => { - if local_key.as_ref().map(|k| k.public() == id).unwrap_or(false) { + if local_key.as_ref().map(|k| k == &id).unwrap_or(false) { HasVoted::Yes(id, vote) } else { HasVoted::No @@ -718,6 +717,7 @@ where }; let (incoming, outgoing) = self.network.round_communication( + self.config.keystore.clone(), crate::communication::Round(round), crate::communication::SetId(self.set_id), self.voters.clone(), @@ -740,7 +740,7 @@ where let outgoing = Box::pin(outgoing.sink_err_into()); voter::RoundData { - voter_id: local_key.map(|pair| pair.public()), + voter_id: local_key, prevote_timer: Box::pin(prevote_timer.map(Ok)), precommit_timer: Box::pin(precommit_timer.map(Ok)), incoming, @@ -749,10 +749,10 @@ where } fn proposed(&self, round: RoundNumber, propose: PrimaryPropose) -> Result<(), Self::Error> { - let local_id = crate::is_voter(&self.voters, &self.config.keystore); + let local_id = crate::is_voter(&self.voters, self.config.keystore.as_ref()); let local_id = match local_id { - Some(id) => id.public(), + Some(id) => id, None => return Ok(()), }; @@ -788,10 +788,10 @@ where } fn prevoted(&self, round: RoundNumber, prevote: Prevote) -> Result<(), Self::Error> { - let local_id = crate::is_voter(&self.voters, &self.config.keystore); + let local_id = crate::is_voter(&self.voters, self.config.keystore.as_ref()); let local_id = match local_id { - Some(id) => id.public(), + Some(id) => id, None => return Ok(()), }; @@ -829,10 +829,10 @@ where } fn precommitted(&self, round: RoundNumber, precommit: Precommit) -> Result<(), Self::Error> { - let local_id = crate::is_voter(&self.voters, &self.config.keystore); + let local_id = crate::is_voter(&self.voters, self.config.keystore.as_ref()); let local_id = match local_id { - Some(id) => id.public(), + Some(id) => id, None => return Ok(()), }; diff --git a/client/finality-grandpa/src/import.rs b/client/finality-grandpa/src/import.rs index 7e3799b1e25e1..b37ab7907a62e 100644 --- a/client/finality-grandpa/src/import.rs +++ b/client/finality-grandpa/src/import.rs @@ -669,6 +669,7 @@ where Error::Blockchain(error) => ConsensusError::ClientImport(error), Error::Client(error) => ConsensusError::ClientImport(error.to_string()), Error::Safety(error) => ConsensusError::ClientImport(error), + Error::Signing(error) => ConsensusError::ClientImport(error), Error::Timer(error) => ConsensusError::ClientImport(error.to_string()), }); }, diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 7b20d082a01ab..481544b5c640d 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -56,8 +56,10 @@ #![warn(missing_docs)] -use futures::prelude::*; -use futures::StreamExt; +use futures::{ + prelude::*, + StreamExt, +}; use log::{debug, info}; use sc_client_api::{ backend::{AuxStore, Backend}, @@ -70,10 +72,13 @@ use sp_api::ProvideRuntimeApi; use sp_blockchain::{HeaderBackend, Error as ClientError, HeaderMetadata}; use sp_runtime::generic::BlockId; use sp_runtime::traits::{NumberFor, Block as BlockT, DigestFor, Zero}; -use sc_keystore::KeyStorePtr; use sp_inherents::InherentDataProviders; use sp_consensus::{SelectChain, BlockImport}; -use sp_core::Pair; +use sp_core::{ + crypto::Public, + traits::BareCryptoStorePtr, +}; +use sp_application_crypto::AppKey; use sp_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; use sc_telemetry::{telemetry, CONSENSUS_INFO, CONSENSUS_DEBUG}; use parking_lot::RwLock; @@ -131,10 +136,10 @@ use aux_schema::PersistentData; use environment::{Environment, VoterSetState}; use until_imported::UntilGlobalMessageBlocksImported; use communication::{NetworkBridge, Network as NetworkT}; -use sp_finality_grandpa::{AuthorityList, AuthorityPair, AuthoritySignature, SetId}; +use sp_finality_grandpa::{AuthorityList, AuthoritySignature, SetId}; // Re-export these two because it's just so damn convenient. -pub use sp_finality_grandpa::{AuthorityId, GrandpaApi, ScheduledChange}; +pub use sp_finality_grandpa::{AuthorityId, AuthorityPair, GrandpaApi, ScheduledChange}; use std::marker::PhantomData; #[cfg(test)] @@ -264,7 +269,7 @@ pub struct Config { /// Some local identifier of the voter. pub name: Option, /// The keystore that manages the keys of this node. - pub keystore: Option, + pub keystore: Option, } impl Config { @@ -284,6 +289,8 @@ pub enum Error { Blockchain(String), /// Could not complete a round on disk. Client(ClientError), + /// Could not sign outgoing message + Signing(String), /// An invariant has been violated (e.g. not finalizing pending change blocks in-order) Safety(String), /// A timer failed to fire. @@ -586,7 +593,7 @@ fn global_communication( voters: &Arc>, client: Arc, network: &NetworkBridge, - keystore: &Option, + keystore: &Option, metrics: Option, ) -> ( impl Stream< @@ -602,7 +609,7 @@ fn global_communication( N: NetworkT, NumberFor: BlockNumberOps, { - let is_voter = is_voter(voters, keystore).is_some(); + let is_voter = is_voter(voters, keystore.as_ref()).is_some(); // verification stream let (global_in, global_out) = network.global_communication( @@ -729,7 +736,7 @@ pub fn run_grandpa_voter( .for_each(move |_| { let curr = authorities.current_authorities(); let mut auths = curr.iter().map(|(p, _)| p); - let maybe_authority_id = authority_id(&mut auths, &conf.keystore) + let maybe_authority_id = authority_id(&mut auths, conf.keystore.as_ref()) .unwrap_or_default(); telemetry!(CONSENSUS_INFO; "afg.authority_set"; @@ -865,8 +872,7 @@ where fn rebuild_voter(&mut self) { debug!(target: "afg", "{}: Starting new voter with set ID {}", self.env.config.name(), self.env.set_id); - let authority_id = is_voter(&self.env.voters, &self.env.config.keystore) - .map(|ap| ap.public()) + let authority_id = is_voter(&self.env.voters, self.env.config.keystore.as_ref()) .unwrap_or_default(); telemetry!(CONSENSUS_DEBUG; "afg.starting_new_voter"; @@ -1089,12 +1095,16 @@ pub fn setup_disabled_grandpa( /// Returns the key pair of the node that is being used in the current voter set or `None`. fn is_voter( voters: &Arc>, - keystore: &Option, -) -> Option { + keystore: Option<&BareCryptoStorePtr>, +) -> Option { match keystore { Some(keystore) => voters .iter() - .find_map(|(p, _)| keystore.read().key_pair::(&p).ok()), + .find(|(p, _)| { + keystore.read() + .has_keys(&[(p.to_raw_vec(), AuthorityId::ID)]) + }) + .map(|(p, _)| p.clone()), None => None, } } @@ -1102,19 +1112,16 @@ fn is_voter( /// Returns the authority id of this node, if available. fn authority_id<'a, I>( authorities: &mut I, - keystore: &Option, + keystore: Option<&BareCryptoStorePtr>, ) -> Option where I: Iterator, { match keystore { Some(keystore) => { authorities - .find_map(|p| { - keystore.read().key_pair::(&p) - .ok() - .map(|ap| ap.public()) - }) - } + .find(|p| keystore.read().has_keys(&[(p.to_raw_vec(), AuthorityId::ID)])) + .cloned() + }, None => None, } } diff --git a/client/finality-grandpa/src/observer.rs b/client/finality-grandpa/src/observer.rs index cd678b3bb4542..f7179d70e7a34 100644 --- a/client/finality-grandpa/src/observer.rs +++ b/client/finality-grandpa/src/observer.rs @@ -26,7 +26,7 @@ use finality_grandpa::{ BlockNumberOps, Error as GrandpaError, voter, voter_set::VoterSet }; use log::{debug, info, warn}; - +use sp_core::traits::BareCryptoStorePtr; use sp_consensus::SelectChain; use sc_client_api::backend::Backend; use sp_utils::mpsc::TracingUnboundedReceiver; @@ -211,7 +211,7 @@ struct ObserverWork> { client: Arc, network: NetworkBridge, persistent_data: PersistentData, - keystore: Option, + keystore: Option, voter_commands_rx: TracingUnboundedReceiver>>, _phantom: PhantomData, } @@ -228,7 +228,7 @@ where client: Arc, network: NetworkBridge, persistent_data: PersistentData, - keystore: Option, + keystore: Option, voter_commands_rx: TracingUnboundedReceiver>>, ) -> Self { @@ -239,7 +239,7 @@ where client, network, persistent_data, - keystore, + keystore: keystore.clone(), voter_commands_rx, _phantom: PhantomData, }; diff --git a/client/finality-grandpa/src/tests.rs b/client/finality-grandpa/src/tests.rs index 25e6253652001..ffd8f1c8c642d 100644 --- a/client/finality-grandpa/src/tests.rs +++ b/client/finality-grandpa/src/tests.rs @@ -282,7 +282,7 @@ fn make_ids(keys: &[Ed25519Keyring]) -> AuthorityList { keys.iter().map(|key| key.clone().public().into()).map(|id| (id, 1)).collect() } -fn create_keystore(authority: Ed25519Keyring) -> (KeyStorePtr, tempfile::TempDir) { +fn create_keystore(authority: Ed25519Keyring) -> (BareCryptoStorePtr, tempfile::TempDir) { let keystore_path = tempfile::tempdir().expect("Creates keystore path"); let keystore = sc_keystore::Store::open(keystore_path.path(), None).expect("Creates keystore"); keystore.write().insert_ephemeral_from_seed::(&authority.to_seed()) @@ -1050,7 +1050,7 @@ fn voter_persists_its_votes() { voter_rx: TracingUnboundedReceiver<()>, net: Arc>, client: PeersClient, - keystore: KeyStorePtr, + keystore: BareCryptoStorePtr, } impl Future for ResettableVoter { @@ -1136,7 +1136,7 @@ fn voter_persists_its_votes() { let config = Config { gossip_duration: TEST_GOSSIP_DURATION, justification_period: 32, - keystore: Some(keystore), + keystore: Some(keystore.clone()), name: Some(format!("peer#{}", 1)), is_authority: true, observer_enabled: true, @@ -1160,10 +1160,11 @@ fn voter_persists_its_votes() { ); let (round_rx, round_tx) = network.round_communication( + Some(keystore), communication::Round(1), communication::SetId(0), Arc::new(VoterSet::new(voters).unwrap()), - Some(peers[1].pair().into()), + Some(peers[1].public().into()), HasVoted::No, ); diff --git a/primitives/finality-grandpa/src/lib.rs b/primitives/finality-grandpa/src/lib.rs index 2e81c8cecbb70..889468a352819 100644 --- a/primitives/finality-grandpa/src/lib.rs +++ b/primitives/finality-grandpa/src/lib.rs @@ -29,6 +29,9 @@ use codec::{Encode, Decode, Input, Codec}; use sp_runtime::{ConsensusEngineId, RuntimeDebug, traits::NumberFor}; use sp_std::borrow::Cow; use sp_std::vec::Vec; +#[cfg(feature = "std")] +use sp_core::traits::BareCryptoStorePtr; +use sp_std::convert::TryInto; #[cfg(feature = "std")] use log::debug; @@ -370,25 +373,31 @@ where /// Localizes the message to the given set and round and signs the payload. #[cfg(feature = "std")] pub fn sign_message( + keystore: BareCryptoStorePtr, message: grandpa::Message, - pair: &AuthorityPair, + public: AuthorityId, round: RoundNumber, set_id: SetId, -) -> grandpa::SignedMessage +) -> Option> where H: Encode, N: Encode, { - use sp_core::Pair; + use sp_core::crypto::Public; + use sp_application_crypto::AppKey; let encoded = localized_payload(round, set_id, &message); - let signature = pair.sign(&encoded[..]); + let signature = keystore.read() + .sign_with(AuthorityId::ID, &public.to_public_crypto_pair(), &encoded[..]) + .ok()? + .try_into() + .ok()?; - grandpa::SignedMessage { + Some(grandpa::SignedMessage { message, signature, - id: pair.public(), - } + id: public, + }) } /// WASM function call to check for pending changes.