From c545344c60a8b78f1b64c00628c254681ddd3c8e Mon Sep 17 00:00:00 2001
From: Ashley <ashley.ruglys@gmail.com>
Date: Tue, 1 Oct 2019 17:42:11 +1300
Subject: [PATCH] Add an Error type to Aura (#3688)
* Add an Error type to Aura
* Add Cargo.lock
* AuRa -> Aura
Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
---
substrate/Cargo.lock | 1 +
substrate/core/consensus/aura/Cargo.toml | 1 +
substrate/core/consensus/aura/src/lib.rs | 67 +++++++++++++++---------
3 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock
index 0787d7faec4..27066afe3c0 100644
--- a/substrate/Cargo.lock
+++ b/substrate/Cargo.lock
@@ -4771,6 +4771,7 @@ dependencies = [
name = "substrate-consensus-aura"
version = "2.0.0"
dependencies = [
+ "derive_more 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
"env_logger 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
"futures 0.1.28 (registry+https://github.com/rust-lang/crates.io-index)",
"futures-preview 0.3.0-alpha.18 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/substrate/core/consensus/aura/Cargo.toml b/substrate/core/consensus/aura/Cargo.toml
index c3aaeb3d56f..724b720f4fe 100644
--- a/substrate/core/consensus/aura/Cargo.toml
+++ b/substrate/core/consensus/aura/Cargo.toml
@@ -26,6 +26,7 @@ futures01 = { package = "futures", version = "0.1" }
futures-timer = "0.3"
parking_lot = "0.9.0"
log = "0.4"
+derive_more = "0.15.0"
[dev-dependencies]
keyring = { package = "substrate-keyring", path = "../../keyring" }
diff --git a/substrate/core/consensus/aura/src/lib.rs b/substrate/core/consensus/aura/src/lib.rs
index a61eb6da721..eb2d1dba20f 100644
--- a/substrate/core/consensus/aura/src/lib.rs
+++ b/substrate/core/consensus/aura/src/lib.rs
@@ -48,7 +48,7 @@ use sr_primitives::{generic::{BlockId, OpaqueDigestItemId}, Justification};
use sr_primitives::traits::{Block as BlockT, Header, DigestItemFor, ProvideRuntimeApi, Zero, Member};
use primitives::crypto::Pair;
-use inherents::{InherentDataProviders, InherentData};
+use inherents::{InherentDataProviders, InherentData, RuntimeString};
use futures::prelude::*;
use parking_lot::Mutex;
@@ -308,16 +308,33 @@ impl<H, B: BlockT, C, E, I, P, Error, SO> SlotWorker<B> for AuraWorker<C, E, I,
}
}
-macro_rules! aura_err {
- ($($i: expr),+) => {
- {
- debug!(target: "aura", $( $i ),+);
- format!($( $i ),+)
- }
- };
+fn aura_err<B: BlockT>(error: Error<B>) -> Error<B> {
+ debug!(target: "aura", "{}", error);
+ error
+}
+
+#[derive(derive_more::Display)]
+enum Error<B: BlockT> {
+ #[display(fmt = "Multiple Aura pre-runtime headers")]
+ MultipleHeaders,
+ #[display(fmt = "No Aura pre-runtime digest found")]
+ NoDigestFound,
+ #[display(fmt = "Header {:?} is unsealed", _0)]
+ HeaderUnsealed(B::Hash),
+ #[display(fmt = "Header {:?} has a bad seal", _0)]
+ HeaderBadSeal(B::Hash),
+ #[display(fmt = "Slot Author not found")]
+ SlotAuthorNotFound,
+ #[display(fmt = "Bad signature on {:?}", _0)]
+ BadSignature(B::Hash),
+ #[display(fmt = "Rejecting block too far in future")]
+ TooFarInFuture,
+ Client(client::error::Error),
+ DataProvider(String),
+ Runtime(RuntimeString)
}
-fn find_pre_digest<B: BlockT, P: Pair>(header: &B::Header) -> Result<u64, String>
+fn find_pre_digest<B: BlockT, P: Pair>(header: &B::Header) -> Result<u64, Error<B>>
where DigestItemFor<B>: CompatibleDigestItem<P>,
P::Signature: Decode,
P::Public: Encode + Decode + PartialEq + Clone,
@@ -326,12 +343,12 @@ fn find_pre_digest<B: BlockT, P: Pair>(header: &B::Header) -> Result<u64, String
for log in header.digest().logs() {
trace!(target: "aura", "Checking log {:?}", log);
match (log.as_aura_pre_digest(), pre_digest.is_some()) {
- (Some(_), true) => Err(aura_err!("Multiple AuRa pre-runtime headers, rejecting!"))?,
+ (Some(_), true) => Err(aura_err(Error::MultipleHeaders))?,
(None, _) => trace!(target: "aura", "Ignoring digest not meant for us"),
(s, false) => pre_digest = s,
}
}
- pre_digest.ok_or_else(|| aura_err!("No AuRa pre-runtime digest found"))
+ pre_digest.ok_or_else(|| aura_err(Error::NoDigestFound))
}
/// check a header has been signed by the right key. If the slot is too far in the future, an error will be returned.
@@ -348,7 +365,7 @@ fn check_header<C, B: BlockT, P: Pair, T>(
hash: B::Hash,
authorities: &[AuthorityId<P>],
_transaction_pool: Option<&T>,
-) -> Result<CheckedHeader<B::Header, (u64, DigestItemFor<B>)>, String> where
+) -> Result<CheckedHeader<B::Header, (u64, DigestItemFor<B>)>, Error<B>> where
DigestItemFor<B>: CompatibleDigestItem<P>,
P::Signature: Decode,
C: client::backend::AuxStore,
@@ -357,11 +374,11 @@ fn check_header<C, B: BlockT, P: Pair, T>(
{
let seal = match header.digest_mut().pop() {
Some(x) => x,
- None => return Err(format!("Header {:?} is unsealed", hash)),
+ None => return Err(Error::HeaderUnsealed(hash)),
};
let sig = seal.as_aura_seal().ok_or_else(|| {
- aura_err!("Header {:?} has a bad seal", hash)
+ aura_err(Error::HeaderBadSeal(hash))
})?;
let slot_num = find_pre_digest::<B, _>(&header)?;
@@ -373,7 +390,7 @@ fn check_header<C, B: BlockT, P: Pair, T>(
// check the signature is valid under the expected authority and
// chain state.
let expected_author = match slot_author::<P>(slot_num, &authorities) {
- None => return Err("Slot Author not found".to_string()),
+ None => return Err(Error::SlotAuthorNotFound),
Some(author) => author,
};
@@ -386,7 +403,7 @@ fn check_header<C, B: BlockT, P: Pair, T>(
slot_num,
&header,
expected_author,
- ).map_err(|e| e.to_string())? {
+ ).map_err(Error::Client)? {
info!(
"Slot author is equivocating at slot {} with headers {:?} and {:?}",
slot_num,
@@ -397,7 +414,7 @@ fn check_header<C, B: BlockT, P: Pair, T>(
Ok(CheckedHeader::Checked(header, (slot_num, seal)))
} else {
- Err(format!("Bad signature on {:?}", hash))
+ Err(Error::BadSignature(hash))
}
}
}
@@ -419,7 +436,7 @@ impl<C, P, T> AuraVerifier<C, P, T>
block_id: BlockId<B>,
inherent_data: InherentData,
timestamp_now: u64,
- ) -> Result<(), String>
+ ) -> Result<(), Error<B>>
where C: ProvideRuntimeApi, C::Api: BlockBuilderApi<B>
{
const MAX_TIMESTAMP_DRIFT_SECS: u64 = 60;
@@ -428,7 +445,7 @@ impl<C, P, T> AuraVerifier<C, P, T>
&block_id,
block,
inherent_data,
- ).map_err(|e| format!("{:?}", e))?;
+ ).map_err(Error::Client)?;
if !inherent_res.ok() {
inherent_res
@@ -438,7 +455,7 @@ impl<C, P, T> AuraVerifier<C, P, T>
// halt import until timestamp is valid.
// reject when too far ahead.
if timestamp > timestamp_now + MAX_TIMESTAMP_DRIFT_SECS {
- return Err("Rejecting block too far in future".into());
+ return Err(Error::TooFarInFuture);
}
let diff = timestamp.saturating_sub(timestamp_now);
@@ -453,8 +470,10 @@ impl<C, P, T> AuraVerifier<C, P, T>
thread::sleep(Duration::from_secs(diff));
Ok(())
},
- Some(TIError::Other(e)) => Err(e.into()),
- None => Err(self.inherent_data_providers.error_to_string(&i, &e)),
+ Some(TIError::Other(e)) => Err(Error::Runtime(e)),
+ None => Err(Error::DataProvider(
+ self.inherent_data_providers.error_to_string(&i, &e)
+ )),
})
} else {
Ok(())
@@ -497,7 +516,7 @@ impl<B: BlockT, C, P, T> Verifier<B> for AuraVerifier<C, P, T> where
hash,
&authorities[..],
self.transaction_pool.as_ref().map(|x| &**x),
- )?;
+ ).map_err(|e| e.to_string())?;
match checked_header {
CheckedHeader::Checked(pre_header, (slot_num, seal)) => {
// if the body is passed through, we need to use the runtime
@@ -518,7 +537,7 @@ impl<B: BlockT, C, P, T> Verifier<B> for AuraVerifier<C, P, T> where
BlockId::Hash(parent_hash),
inherent_data,
timestamp_now,
- )?;
+ ).map_err(|e| e.to_string())?;
}
let (_, inner_body) = block.deconstruct();
--
GitLab