Remove dependency on error_chain (#277)

* Convert validation error

* Convert wasm_executor error

* Convert block evaluation error

* Convert collation errors and the compilation

* Remove error-chain dep from service

* Remove unused Result type

* Remove unused error variants

* Remove redundant intos

* Add missing comments

* Update validation/src/collation.rs

Co-Authored-By: thiolliere <gui.thiolliere@gmail.com>

* Fix new error variant

polkadot/validation/src/collation.rs

@@ -27,10 +27,8 @@ use polkadot_primitives::{Block, Hash, BlockId, parachain::CollatorId, parachain
 }};
 use runtime_primitives::traits::ProvideRuntimeApi;
 use parachain::{wasm_executor::{self, ExternalitiesError}, MessageRef, UpwardMessageRef};
-use error_chain::bail;
 
 use futures::prelude::*;
-use error_chain::*;
 use log::debug;
 
 /// Encapsulates connections to collators and allows collation on any parachain.
@@ -136,62 +134,47 @@ impl<C: Collators, P: ProvideRuntimeApi> Future for CollationFetch<C, P>
 }
 
 // Errors that can occur when validating a parachain.
-error_chain! {
-	types { Error, ErrorKind, ResultExt; }
+#[derive(Debug, derive_more::Display, derive_more::From)]
+pub enum Error {
+	/// Client error
+	Client(client::error::Error),
+	/// Wasm validation error
+	WasmValidation(wasm_executor::Error),
+	/// Collated for inactive parachain
+	#[display(fmt = "Collated for inactive parachain: {:?}", _0)]
+	InactiveParachain(ParaId),
+	/// Unexpected egress root
+	#[display(fmt = "Got unexpected egress root to {:?}. (expected: {:?}, got {:?})", id, expected, got)]
+	EgressRootMismatch { id: ParaId, expected: Hash, got: Hash },
+	/// Unexpected ingress root
+	#[display(fmt = "Got unexpected ingress root to {:?}. (expected: {:?}, got {:?})", id, expected, got)]
+	IngressRootMismatch { id: ParaId, expected: Hash, got: Hash },
+	/// Ingress from wrong chain
+	#[display(fmt = "Got ingress from wrong chain. (expected: {:?}, got {:?})", expected, got)]
+	IngressChainMismatch { expected: ParaId, got: ParaId },
+	/// Ingress canonicality mismatch
+	#[display(fmt = "Got data for {} roots, expected {}", expected, got)]
+	IngressCanonicalityMismatch { expected: usize, got: usize },
+	/// Missing or extra egress root
+	#[display(fmt = "Missing or extra egress root. (expected: {:?}, got {:?})", expected, got)]
+	MissingEgressRoot { expected: Option<ParaId>, got: Option<ParaId>, },
+	/// Parachain validation produced wrong head data
+	#[display(fmt = "Parachain validation produced wrong head data (expected: {:?}, got {:?})", expected, got)]
+	WrongHeadData { expected: Vec<u8>, got: Vec<u8> },
+	/// Block data is too big
+	#[display(fmt = "Block data is too big (maximum allowed size: {}, actual size: {})", size, max_size)]
+	BlockDataTooBig { size: u64, max_size: u64 },
+	/// Parachain validation produced wrong relay-chain messages
+	#[display(fmt = "Parachain validation produced wrong relay-chain messages (expected: {:?}, got {:?})", expected, got)]
+	UpwardMessagesInvalid { expected: Vec<UpwardMessage>, got: Vec<UpwardMessage> },
+}
 
-	foreign_links {
-		Client(::client::error::Error);
-	}
-
-	links {
-		WasmValidation(wasm_executor::Error, wasm_executor::ErrorKind);
-	}
-
-	errors {
-		InactiveParachain(id: ParaId) {
-			description("Collated for inactive parachain"),
-			display("Collated for inactive parachain: {:?}", id),
-		}
-		EgressRootMismatch(id: ParaId, expected: Hash, got: Hash) {
-			description("Got unexpected egress root."),
-			display(
-				"Got unexpected egress root to {:?}. (expected: {:?}, got {:?})",
-				id, expected, got
-			),
-		}
-		IngressRootMismatch(id: ParaId, expected: Hash, got: Hash) {
-			description("Got unexpected ingress root."),
-			display(
-				"Got unexpected ingress root to {:?}. (expected: {:?}, got {:?})",
-				id, expected, got
-			),
-		}
-		IngressChainMismatch(expected: ParaId, got: ParaId) {
-			description("Got ingress from wrong chain"),
-			display(
-				"Got ingress from wrong chain. (expected: {:?}, got {:?})",
-				expected, got
-			),
-		}
-		IngressCanonicalityMismatch(expected: usize, got: usize) {
-			description("Ingress canonicality mismatch."),
-			display("Got data for {} roots, expected {}", got, expected),
-		}
-		MissingEgressRoot(expected: Option<ParaId>, got: Option<ParaId>) {
-			description("Missing or extra egress root."),
-			display("Missing or extra egress root. (expected: {:?}, got {:?})", expected, got),
-		}
-		WrongHeadData(expected: Vec<u8>, got: Vec<u8>) {
-			description("Parachain validation produced wrong head data."),
-			display("Parachain validation produced wrong head data (expected: {:?}, got {:?})", expected, got),
-		}
-		BlockDataTooBig(size: u64, max_size: u64) {
-			description("Block data is too big."),
-			display("Block data is too big (maximum allowed size: {}, actual size: {})", max_size, size),
-		}
-		UpwardMessagesInvalid(expected: Vec<UpwardMessage>, got: Vec<UpwardMessage>) {
-			description("Parachain validation produced wrong relay-chain messages."),
-			display("Parachain validation produced wrong relay-chain messages (expected: {:?}, got {:?})", expected, got),
+impl std::error::Error for Error {
+	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+		match self {
+			Error::Client(ref err) => Some(err),
+			Error::WasmValidation(ref err) => Some(err),
+			_ => None,
 		}
 	}
 }
@@ -240,11 +223,17 @@ fn check_extrinsic(
 		let mut expected_egress_roots = expected_egress_roots.iter();
 		while let Some(batch_target) = messages_iter.peek().map(|o| o.target) {
 			let expected_root = match expected_egress_roots.next() {
-				None => return Err(ErrorKind::MissingEgressRoot(Some(batch_target), None).into()),
+				None => return Err(Error::MissingEgressRoot {
+					expected: Some(batch_target),
+					got: None
+				}),
 				Some(&(id, ref root)) => if id == batch_target {
 					root
 				} else {
-					return Err(ErrorKind::MissingEgressRoot(Some(batch_target), Some(id)).into());
+					return Err(Error::MissingEgressRoot{
+						expected: Some(batch_target),
+						got: Some(id)
+					});
 				}
 			};
 
@@ -258,17 +247,17 @@ fn check_extrinsic(
 
 			let computed_root = message_queue_root(messages_to);
 			if &computed_root != expected_root {
-				return Err(ErrorKind::EgressRootMismatch(
-					batch_target,
-					expected_root.clone(),
-					computed_root,
-				).into());
+				return Err(Error::EgressRootMismatch {
+					id: batch_target,
+					expected: expected_root.clone(),
+					got: computed_root,
+				});
 			}
 		}
 
 		// also check that there are no more additional expected roots.
 		if let Some((next_target, _)) = expected_egress_roots.next() {
-			return Err(ErrorKind::MissingEgressRoot(None, Some(*next_target)).into());
+			return Err(Error::MissingEgressRoot { expected: None, got: Some(*next_target) });
 		}
 	}
 
@@ -320,10 +309,10 @@ impl Externalities {
 		candidate: &CandidateReceipt,
 	) -> Result<Extrinsic, Error> {
 		if &self.upward != &candidate.upward_messages {
-			bail!(ErrorKind::UpwardMessagesInvalid(
-				candidate.upward_messages.clone(),
-				self.upward.clone(),
-			));
+			return Err(Error::UpwardMessagesInvalid {
+				expected: candidate.upward_messages.clone(),
+				got: self.upward.clone(),
+			});
 		}
 
 		check_extrinsic(
@@ -339,18 +328,28 @@ pub fn validate_incoming(
 	ingress: &ConsolidatedIngress,
 ) -> Result<(), Error> {
 	if roots.0.len() != ingress.0.len() {
-		bail!(ErrorKind::IngressCanonicalityMismatch(roots.0.len(), ingress.0.len()));
+		return Err(Error::IngressCanonicalityMismatch {
+			expected: roots.0.len(),
+			got: ingress.0.len()
+		});
 	}
 
 	let all_iter = roots.0.iter().zip(&ingress.0);
 	for ((expected_id, root), (got_id, messages)) in all_iter {
 		if expected_id != got_id {
-			bail!(ErrorKind::IngressChainMismatch(*expected_id, *got_id));
+			return Err(Error::IngressChainMismatch {
+				expected: *expected_id,
+				got: *got_id
+			});
 		}
 
 		let got_root = message_queue_root(messages.iter().map(|msg| &msg.0[..]));
 		if &got_root != root {
-			bail!(ErrorKind::IngressRootMismatch(*expected_id, *root, got_root));
+			return Err(Error::IngressRootMismatch{
+				id: *expected_id,
+				expected: *root,
+				got: got_root
+			});
 		}
 	}
 
@@ -375,20 +374,20 @@ pub fn validate_collation<P>(
 	if let Some(max_size) = max_block_data_size {
 		let block_data_size = collation.pov.block_data.0.len() as u64;
 		if block_data_size > max_size {
-			return Err(ErrorKind::BlockDataTooBig(block_data_size, max_size).into());
+			return Err(Error::BlockDataTooBig { size: block_data_size, max_size });
 		}
 	}
 
 	let api = client.runtime_api();
 	let para_id = collation.receipt.parachain_index;
 	let validation_code = api.parachain_code(relay_parent, para_id)?
-		.ok_or_else(|| ErrorKind::InactiveParachain(para_id))?;
+		.ok_or_else(|| Error::InactiveParachain(para_id))?;
 
 	let chain_head = api.parachain_head(relay_parent, para_id)?
-		.ok_or_else(|| ErrorKind::InactiveParachain(para_id))?;
+		.ok_or_else(|| Error::InactiveParachain(para_id))?;
 
 	let roots = api.ingress(relay_parent, para_id)?
-		.ok_or_else(|| ErrorKind::InactiveParachain(para_id))?;
+		.ok_or_else(|| Error::InactiveParachain(para_id))?;
 	validate_incoming(&roots, &collation.pov.ingress)?;
 
 	let params = ValidationParams {
@@ -415,10 +414,10 @@ pub fn validate_collation<P>(
 			if result.head_data == collation.receipt.head_data.0 {
 				ext.final_checks(&collation.receipt)
 			} else {
-				Err(ErrorKind::WrongHeadData(
-					collation.receipt.head_data.0.clone(),
-					result.head_data
-				).into())
+				Err(Error::WrongHeadData {
+					expected: collation.receipt.head_data.0.clone(),
+					got: result.head_data
+				})
 			}
 		}
 		Err(e) => Err(e.into())

polkadot/validation/src/error.rs

@@ -18,38 +18,44 @@
 
 use runtime_primitives::RuntimeString;
 use primitives::ed25519::Public as AuthorityId;
-use error_chain::*;
 
-error_chain! {
-	foreign_links {
-		Client(::client::error::Error);
-		Consensus(::consensus::error::Error);
-	}
+/// Error type for validation
+#[derive(Debug, derive_more::Display, derive_more::From)]
+pub enum Error {
+	/// Client error
+	Client(client::error::Error),
+	/// Consensus error
+	Consensus(consensus::error::Error),
+	#[display(fmt = "Invalid duty roster length: expected {}, got {}", expected, got)]
+	InvalidDutyRosterLength {
+		/// Expected roster length
+		expected: usize,
+		/// Actual roster length
+		got: usize,
+	},
+	/// Local account not a validator at this block
+	#[display(fmt = "Local account ID ({:?}) not a validator at this block.", _0)]
+	NotValidator(AuthorityId),
+	/// Unexpected error checking inherents
+	#[display(fmt = "Unexpected error while checking inherents: {}", _0)]
+	InherentError(RuntimeString),
+	/// Proposer destroyed before finishing proposing or evaluating
+	#[display(fmt = "Proposer destroyed before finishing proposing or evaluating")]
+	PrematureDestruction,
+	/// Timer failed
+	#[display(fmt = "Timer failed: {}", _0)]
+	Timer(tokio::timer::Error),
+	/// Unable to dispatch agreement future
+	#[display(fmt = "Unable to dispatch agreement future: {:?}", _0)]
+	Executor(futures::future::ExecuteErrorKind),
+}
 
-	errors {
-		InvalidDutyRosterLength(expected: usize, got: usize) {
-			description("Duty Roster had invalid length"),
-			display("Invalid duty roster length: expected {}, got {}", expected, got),
-		}
-		NotValidator(id: AuthorityId) {
-			description("Local account ID not a validator at this block."),
-			display("Local account ID ({:?}) not a validator at this block.", id),
-		}
-		InherentError(reason: RuntimeString) {
-			description("Unexpected error while checking inherents"),
-			display("Unexpected error while checking inherents: {}", reason),
-		}
-		PrematureDestruction {
-			description("Proposer destroyed before finishing proposing or evaluating"),
-			display("Proposer destroyed before finishing proposing or evaluating"),
-		}
-		Timer(e: ::tokio::timer::Error) {
-			description("Failed to register or resolve async timer."),
-			display("Timer failed: {}", e),
-		}
-		Executor(e: ::futures::future::ExecuteErrorKind) {
-			description("Unable to dispatch agreement future"),
-			display("Unable to dispatch agreement future: {:?}", e),
+impl std::error::Error for Error {
+	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+		match self {
+			Error::Client(ref err) => Some(err),
+			Error::Consensus(ref err) => Some(err),
+			_ => None,
 		}
 	}
 }

polkadot/validation/src/evaluation.rs

@@ -21,44 +21,40 @@ use super::MAX_TRANSACTIONS_SIZE;
 use parity_codec::Encode;
 use polkadot_primitives::{Block, Hash, BlockNumber};
 use polkadot_primitives::parachain::Id as ParaId;
-use error_chain::*;
 
-error_chain! {
-	foreign_links {
-		Client(::client::error::Error);
-	}
+/// Result type alias for block evaluation
+pub type Result<T> = std::result::Result<T, Error>;
 
-	errors {
-		ProposalNotForPolkadot {
-			description("Proposal provided not a Polkadot block."),
-			display("Proposal provided not a Polkadot block."),
-		}
-		TooManyCandidates(expected: usize, got: usize) {
-			description("Proposal included more candidates than is possible."),
-			display("Proposal included {} candidates for {} parachains", got, expected),
-		}
-		ParachainOutOfOrder {
-			description("Proposal included parachains out of order."),
-			display("Proposal included parachains out of order."),
-		}
-		UnknownParachain(id: ParaId) {
-			description("Proposal included unregistered parachain."),
-			display("Proposal included unregistered parachain {:?}", id),
-		}
-		WrongParentHash(expected: Hash, got: Hash) {
-			description("Proposal had wrong parent hash."),
-			display("Proposal had wrong parent hash. Expected {:?}, got {:?}", expected, got),
-		}
-		WrongNumber(expected: BlockNumber, got: BlockNumber) {
-			description("Proposal had wrong number."),
-			display("Proposal had wrong number. Expected {:?}, got {:?}", expected, got),
-		}
-		ProposalTooLarge(size: usize) {
-			description("Proposal exceeded the maximum size."),
-			display(
-				"Proposal exceeded the maximum size of {} by {} bytes.",
-				MAX_TRANSACTIONS_SIZE, MAX_TRANSACTIONS_SIZE.saturating_sub(*size)
-			),
+/// Error type for block evaluation
+#[derive(Debug, derive_more::Display, derive_more::From)]
+pub enum Error {
+	/// Client error
+	Client(client::error::Error),
+	/// Too many parachain candidates in proposal
+	#[display(fmt = "Proposal included {} candidates for {} parachains", expected, got)]
+	TooManyCandidates { expected: usize, got: usize },
+	/// Proposal included unregistered parachain
+	#[display(fmt = "Proposal included unregistered parachain {:?}", _0)]
+	UnknownParachain(ParaId),
+	/// Proposal had wrong parent hash
+	#[display(fmt = "Proposal had wrong parent hash. Expected {:?}, got {:?}", expected, got)]
+	WrongParentHash { expected: Hash, got: Hash },
+	/// Proposal had wrong number
+	#[display(fmt = "Proposal had wrong number. Expected {:?}, got {:?}", expected, got)]
+	WrongNumber { expected: BlockNumber, got: BlockNumber },
+	/// Proposal exceeded the maximum size
+	#[display(
+		fmt = "Proposal exceeded the maximum size of {} by {} bytes.",
+		MAX_TRANSACTIONS_SIZE, MAX_TRANSACTIONS_SIZE.saturating_sub(*_0)
+	)]
+	ProposalTooLarge(usize),
+}
+
+impl std::error::Error for Error {
+	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+		match self {
+			Error::Client(ref err) => Some(err),
+			_ => None,
 		}
 	}
 }
@@ -77,15 +73,21 @@ pub fn evaluate_initial(
 	});
 
 	if transactions_size > MAX_TRANSACTIONS_SIZE {
-		bail!(ErrorKind::ProposalTooLarge(transactions_size))
+		return Err(Error::ProposalTooLarge(transactions_size))
 	}
 
 	if proposal.header.parent_hash != *parent_hash {
-		bail!(ErrorKind::WrongParentHash(*parent_hash, proposal.header.parent_hash));
+		return Err(Error::WrongParentHash {
+			expected: *parent_hash,
+			got: proposal.header.parent_hash
+		});
 	}
 
 	if proposal.header.number != parent_number + 1 {
-		bail!(ErrorKind::WrongNumber(parent_number + 1, proposal.header.number));
+		return Err(Error::WrongNumber {
+			expected: parent_number + 1,
+			got: proposal.header.number
+		});
 	}
 
 	Ok(())

polkadot/validation/src/lib.rs

@@ -62,14 +62,13 @@ use dynamic_inclusion::DynamicInclusion;
 use inherents::InherentData;
 use runtime_aura::timestamp::TimestampInherentData;
 use log::{info, debug, warn, trace};
-use error_chain::bail;
 
 use ed25519::Public as AuthorityId;
 
 pub use self::collation::{
 	validate_collation, validate_incoming, message_queue_root, egress_roots, Collators,
 };
-pub use self::error::{ErrorKind, Error};
+pub use self::error::Error;
 pub use self::shared_table::{
 	SharedTable, ParachainWork, PrimedParachainWork, Validated, Statement, SignedStatement,
 	GenericStatement,
@@ -188,7 +187,10 @@ pub fn make_group_info(
 	local_id: AuthorityId,
 ) -> Result<(HashMap<ParaId, GroupInfo>, LocalDuty), Error> {
 	if roster.validator_duty.len() != authorities.len() {
-		bail!(ErrorKind::InvalidDutyRosterLength(authorities.len(), roster.validator_duty.len()))
+		return Err(Error::InvalidDutyRosterLength {
+			expected: authorities.len(),
+			got: roster.validator_duty.len()
+		});
 	}
 
 	let mut local_validation = None;
@@ -223,7 +225,7 @@ pub fn make_group_info(
 
 			Ok((map, local_duty))
 		}
-		None => bail!(ErrorKind::NotValidator(local_id)),
+		None => return Err(Error::NotValidator(local_id)),
 	}
 }
 
@@ -594,7 +596,7 @@ impl<C, TxApi> consensus::Proposer<Block> for Proposer<C, TxApi> where
 
 		let believed_timestamp = match inherent_data.timestamp_inherent_data() {
 			Ok(timestamp) => timestamp,
-			Err(e) => return Either::B(future::err(ErrorKind::InherentError(e).into())),
+			Err(e) => return Either::B(future::err(Error::InherentError(e))),
 		};
 
 		// set up delay until next allowed timestamp.
@@ -649,26 +651,26 @@ struct ProposalTiming {
 impl ProposalTiming {
 	// whether it's time to attempt a proposal.
 	// shouldn't be called outside of the context of a task.
-	fn poll(&mut self, included: usize) -> Poll<(), ErrorKind> {
+	fn poll(&mut self, included: usize) -> Poll<(), Error> {
 		// first drain from the interval so when the minimum delay is up
 		// we don't have any notifications built up.
 		//
 		// this interval is just meant to produce periodic task wakeups
 		// that lead to the `dynamic_inclusion` getting updated as necessary.
-		while let Async::Ready(x) = self.attempt_propose.poll().map_err(ErrorKind::Timer)? {
+		while let Async::Ready(x) = self.attempt_propose.poll().map_err(Error::Timer)? {
 			x.expect("timer still alive; intervals never end; qed");
 		}
 
 		// wait until the minimum time has passed.
 		if let Some(mut minimum) = self.minimum.take() {
-			if let Async::NotReady = minimum.poll().map_err(ErrorKind::Timer)? {
+			if let Async::NotReady = minimum.poll().map_err(Error::Timer)? {
 				self.minimum = Some(minimum);
 				return Ok(Async::NotReady);
 			}
 		}
 
 		if included == self.last_included {
-			return self.enough_candidates.poll().map_err(ErrorKind::Timer);
+			return self.enough_candidates.poll().map_err(Error::Timer);
 		}
 
 		// the amount of includable candidates has changed. schedule a wakeup
@@ -677,7 +679,7 @@ impl ProposalTiming {
 			Some(instant) => {
 				self.last_included = included;
 				self.enough_candidates.reset(instant);
-				self.enough_candidates.poll().map_err(ErrorKind::Timer)
+				self.enough_candidates.poll().map_err(Error::Timer)
 			}
 			None => Ok(Async::Ready(())),
 		}
@@ -712,7 +714,7 @@ impl<C, TxApi> CreateProposal<C, TxApi> where
 		let mut inherent_data = self.inherent_data
 			.take()
 			.expect("CreateProposal is not polled after finishing; qed");
-		inherent_data.put_data(polkadot_runtime::PARACHAIN_INHERENT_IDENTIFIER, &candidates).map_err(ErrorKind::InherentError)?;
+		inherent_data.put_data(polkadot_runtime::PARACHAIN_INHERENT_IDENTIFIER, &candidates).map_err(Error::InherentError)?;
 
 		let runtime_api = self.client.runtime_api();