Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Keystore overhaul (final) #13683

Merged
merged 15 commits into from
Mar 24, 2023
Prev Previous commit
Next Next commit
Signing error revisited for babe and aura as well
  • Loading branch information
davxy committed Mar 24, 2023
commit de251c559113e807cd7e987ae8d91d5d2c52a36f
36 changes: 16 additions & 20 deletions client/consensus/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub fn start_aura<P, B, C, SC, I, PF, SO, L, CIDP, BS, Error>(
telemetry,
compatibility_mode,
}: StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS, NumberFor<B>>,
) -> Result<impl Future<Output = ()>, sp_consensus::Error>
) -> Result<impl Future<Output = ()>, ConsensusError>
where
P: Pair + Send + Sync,
P::Public: AppPublic + Hash + Member + Encode + Decode,
Expand All @@ -222,7 +222,7 @@ where
CIDP: CreateInherentDataProviders<B, ()> + Send + 'static,
CIDP::InherentDataProviders: InherentDataProviderExt + Send,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + Sync + 'static,
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
Error: std::error::Error + Send + From<ConsensusError> + 'static,
{
let worker = build_aura_worker::<P, _, _, _, _, _, _, _, _>(BuildAuraWorkerParams {
client,
Expand Down Expand Up @@ -320,7 +320,7 @@ where
P::Public: AppPublic + Hash + Member + Encode + Decode,
P::Signature: TryFrom<Vec<u8>> + Hash + Member + Encode + Decode,
I: BlockImport<B, Transaction = sp_api::TransactionFor<C, B>> + Send + Sync + 'static,
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
Error: std::error::Error + Send + From<ConsensusError> + 'static,
SO: SyncOracle + Send + Sync + Clone,
L: sc_consensus::JustificationSyncLink<B>,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + Sync + 'static,
Expand Down Expand Up @@ -374,13 +374,13 @@ where
SO: SyncOracle + Send + Clone + Sync,
L: sc_consensus::JustificationSyncLink<B>,
BS: BackoffAuthoringBlocksStrategy<NumberFor<B>> + Send + Sync + 'static,
Error: std::error::Error + Send + From<sp_consensus::Error> + 'static,
Error: std::error::Error + Send + From<ConsensusError> + 'static,
{
type BlockImport = I;
type SyncOracle = SO;
type JustificationSyncLink = L;
type CreateProposer =
Pin<Box<dyn Future<Output = Result<E::Proposer, sp_consensus::Error>> + Send + 'static>>;
Pin<Box<dyn Future<Output = Result<E::Proposer, ConsensusError>> + Send + 'static>>;
type Proposer = E::Proposer;
type Claim = P::Public;
type AuxData = Vec<AuthorityId<P>>;
Expand All @@ -393,11 +393,7 @@ where
&mut self.block_import
}

fn aux_data(
&self,
header: &B::Header,
_slot: Slot,
) -> Result<Self::AuxData, sp_consensus::Error> {
fn aux_data(&self, header: &B::Header, _slot: Slot) -> Result<Self::AuxData, ConsensusError> {
authorities(
self.client.as_ref(),
header.hash(),
Expand Down Expand Up @@ -443,7 +439,7 @@ where
_authorities: Self::AuxData,
) -> Result<
sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
sp_consensus::Error,
ConsensusError,
> {
let public = public.to_raw_vec();
let signature = self
Expand All @@ -454,17 +450,17 @@ where
&public,
header_hash.as_ref(),
)
.map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))?
.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
.ok_or_else(|| {
sp_consensus::Error::CannotSign(
public.clone(),
"Could not find key in keystore.".into(),
)
ConsensusError::CannotSign(format!(
"Could not find key in keystore. Key: {:?}",
public
))
})?;
let signature = signature
.clone()
.try_into()
.map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?;
.map_err(|_| ConsensusError::InvalidSignature(signature, public))?;

let signature_digest_item =
<DigestItem as CompatibleDigestItem<P::Signature>>::aura_seal(signature);
Expand Down Expand Up @@ -509,7 +505,7 @@ where
fn proposer(&mut self, block: &B::Header) -> Self::CreateProposer {
self.env
.init(block)
.map_err(|e| sp_consensus::Error::ClientImport(format!("{:?}", e)))
.map_err(|e| ConsensusError::ClientImport(format!("{:?}", e)))
.boxed()
}

Expand Down Expand Up @@ -622,14 +618,14 @@ where
Default::default(),
),
)
.map_err(|_| sp_consensus::Error::InvalidAuthoritiesSet)?;
.map_err(|_| ConsensusError::InvalidAuthoritiesSet)?;
},
}

runtime_api
.authorities(parent_hash)
.ok()
.ok_or(sp_consensus::Error::InvalidAuthoritiesSet)
.ok_or(ConsensusError::InvalidAuthoritiesSet)
}

#[cfg(test)]
Expand Down
21 changes: 7 additions & 14 deletions client/consensus/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,7 @@ use sp_blockchain::{
use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
use sp_consensus_babe::inherents::BabeInherentData;
use sp_consensus_slots::Slot;
use sp_core::{
crypto::{ByteArray, Wraps},
ExecutionContext,
};
use sp_core::{crypto::Wraps, ExecutionContext};
use sp_inherents::{CreateInherentDataProviders, InherentData, InherentDataProvider};
use sp_keystore::KeystorePtr;
use sp_runtime::{
Expand Down Expand Up @@ -840,12 +837,12 @@ where
public.as_inner_ref(),
header_hash.as_ref(),
)
.map_err(|e| ConsensusError::CannotSign(public.to_raw_vec(), e.to_string()))?
.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
.ok_or_else(|| {
ConsensusError::CannotSign(
public.to_raw_vec(),
"Could not find key in keystore.".into(),
)
ConsensusError::CannotSign(format!(
"Could not find key in keystore. Key: {:?}",
public
))
})?;

let digest_item = <DigestItem as CompatibleDigestItem>::babe_seal(signature.into());
Expand Down Expand Up @@ -891,11 +888,7 @@ where
}

fn proposer(&mut self, block: &B::Header) -> Self::CreateProposer {
Box::pin(
self.env
.init(block)
.map_err(|e| ConsensusError::ClientImport(e.to_string())),
)
Box::pin(self.env.init(block).map_err(|e| ConsensusError::ClientImport(e.to_string())))
}

fn telemetry(&self) -> Option<TelemetryHandle> {
Expand Down
4 changes: 2 additions & 2 deletions primitives/consensus/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ pub enum Error {
#[error("Chain lookup failed: {0}")]
ChainLookup(String),
/// Signing failed.
#[error("Failed to sign using key: {0:?}. Reason: {1}")]
CannotSign(Vec<u8>, String),
#[error("Failed to sign: {0}")]
CannotSign(String),
/// Some other error.
#[error(transparent)]
Other(#[from] Box<dyn std::error::Error + Sync + Send + 'static>),
Expand Down