From 77f06be7fbc431e7532c73c2029f1fd9cfaffa6b Mon Sep 17 00:00:00 2001 From: keorn Date: Wed, 24 Aug 2016 15:55:47 +0200 Subject: [PATCH] fix error propagation --- ethcore/res/bft.json | 39 ----------------------- ethcore/src/client/client.rs | 16 +++++----- ethcore/src/client/test_client.rs | 2 +- ethcore/src/client/traits.rs | 2 +- ethcore/src/engines/mod.rs | 19 ++++++++++-- ethcore/src/engines/propose_collect.rs | 4 +-- ethcore/src/engines/signed_vote.rs | 10 ++---- ethcore/src/engines/tendermint.rs | 43 ++++++++++++++++---------- ethcore/src/error.rs | 12 +++---- 9 files changed, 63 insertions(+), 84 deletions(-) delete mode 100644 ethcore/res/bft.json diff --git a/ethcore/res/bft.json b/ethcore/res/bft.json deleted file mode 100644 index 24bd386b2..000000000 --- a/ethcore/res/bft.json +++ /dev/null @@ -1,39 +0,0 @@ -{ - "name": "TestBFT", - "engine": { - "BFT": { - "params": { - "gasLimitBoundDivisor": "0x0400", - "durationLimit": "0x0d", - "validators" : ["0x9cce34f7ab185c7aba1b7c8140d620b4bda941d6"] - } - } - }, - "params": { - "accountStartNonce": "0x0100000", - "maximumExtraDataSize": "0x20", - "minGasLimit": "0x1388", - "networkID" : "0x69" - }, - "genesis": { - "seal": { - "generic": { - "fields": 1, - "rlp": "0x11bbe8db4e347b4e8c937c1c8370e4b5ed33adb3db69cbdb7a38e1e50b1b82fa" - } - }, - "difficulty": "0x20000", - "author": "0x0000000000000000000000000000000000000000", - "timestamp": "0x00", - "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000", - "extraData": "0x", - "gasLimit": "0x2fefd8" - }, - "accounts": { - "0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, - "0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } }, - "0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } }, - "0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } }, - "9cce34f7ab185c7aba1b7c8140d620b4bda941d6": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" } - } -} diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 51376fc0a..87367bf98 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1024,13 +1024,15 @@ impl BlockChainClient for Client { fn queue_infinity_message(&self, message: Bytes) { let full_rlp = UntrustedRlp::new(&message); - let signature = full_rlp.val_at(0).unwrap_or_else(|| return); - let message: Vec<_> = full_rlp.val_at(1).unwrap_or_else(|| return); - let message_rlp = UntrustedRlp::new(&message); - let pub_key = recover(&signature, &message.sha3()).unwrap_or_else(|| return); - if let Some(new_message) = self.engine.handle_message(pub_key.sha3().into(), message_rlp) - { - self.notify(|notify| notify.broadcast(&new_message)); + if let Ok(signature) = full_rlp.val_at(0) { + if let Ok(message) = full_rlp.val_at::>(1) { + let message_rlp = UntrustedRlp::new(&message); + if let Ok(pub_key) = recover(&signature, &message.sha3()) { + if let Ok(new_message) = self.engine.handle_message(pub_key.sha3().into(), message_rlp) { + self.notify(|notify| notify.broadcast(&new_message)); + } + } + } } } } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 8852448cc..a7d710da3 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -38,7 +38,7 @@ use spec::Spec; use block_queue::BlockQueueInfo; use block::{OpenBlock, SealedBlock}; use executive::Executed; -use error::CallError; +use error::{Error, CallError}; use trace::LocalizedTrace; /// Test client. diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index da876efa6..368bae7d6 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -183,7 +183,7 @@ pub trait BlockChainClient : Sync + Send { fn queue_transactions(&self, transactions: Vec); /// Queue packet - fn queue_infinity_message(&self, packet: Bytes); + fn queue_infinity_message(&self, message: Bytes); /// list all transactions fn pending_transactions(&self) -> Vec; diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index ef7590540..07d17399a 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -27,8 +27,8 @@ pub use self::null_engine::NullEngine; pub use self::instant_seal::InstantSeal; pub use self::basic_authority::BasicAuthority; pub use self::tendermint::Tendermint; -pub use self::signed_vote::{SignedVote, VoteError}; -pub use self::propose_collect::{ProposeCollect}; +pub use self::signed_vote::SignedVote; +pub use self::propose_collect::ProposeCollect; use common::{HashMap, SemanticVersion, Header, EnvInfo, Address, Builtin, BTreeMap, U256, Bytes, SignedTransaction, Error, UntrustedRlp}; use account_provider::AccountProvider; @@ -36,6 +36,19 @@ use block::ExecutedBlock; use spec::CommonParams; use evm::Schedule; +/// Voting errors. +#[derive(Debug)] +pub enum EngineError { + /// Voter is not in the voters set. + UnauthorisedVoter, + /// Message pertaining incorrect consensus step. + WrongStep, + /// Message pertaining unknown consensus step. + UnknownStep, + /// Message was not expected. + UnexpectedMessage +} + /// A consensus mechanism for the chain. Generally either proof-of-work or proof-of-stake-based. /// Provides hooks into each of the major parts of block import. pub trait Engine : Sync + Send { @@ -121,7 +134,7 @@ pub trait Engine : Sync + Send { /// Handle any potential consensus messages; /// updating consensus state and potentially issuing a new one. - fn handle_message(&self, sender: Address, message: UntrustedRlp) -> Option { None } + fn handle_message(&self, sender: Address, message: UntrustedRlp) -> Result { Err(EngineError::UnexpectedMessage.into()) } // TODO: builtin contract routing - to do this properly, it will require removing the built-in configuration-reading logic // from Spec into here and removing the Spec::builtins field. diff --git a/ethcore/src/engines/propose_collect.rs b/ethcore/src/engines/propose_collect.rs index f09618c50..f94132e7d 100644 --- a/ethcore/src/engines/propose_collect.rs +++ b/ethcore/src/engines/propose_collect.rs @@ -18,7 +18,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use common::{HashSet, RwLock, H256, Signature, Address, Error, ec, Hashable}; -use engines::VoteError; +use super::EngineError; /// Collect votes on a hash. #[derive(Debug)] @@ -59,7 +59,7 @@ impl ProposeCollect { fn can_vote(&self, signature: &Signature) -> Result<(), Error> { let signer = Address::from(try!(ec::recover(&signature, &self.hash)).sha3()); match self.voters.contains(&signer) { - false => try!(Err(VoteError::UnauthorisedVoter)), + false => try!(Err(EngineError::UnauthorisedVoter)), true => Ok(()), } } diff --git a/ethcore/src/engines/signed_vote.rs b/ethcore/src/engines/signed_vote.rs index d3381112a..b7c5082a3 100644 --- a/ethcore/src/engines/signed_vote.rs +++ b/ethcore/src/engines/signed_vote.rs @@ -16,6 +16,7 @@ //! Voting on hashes, where each vote has to come from a set of public keys. +use super::EngineError; use common::{HashSet, HashMap, RwLock, H256, Signature, Address, Error, ec, Hashable}; /// Signed voting on hashes. @@ -33,13 +34,6 @@ pub struct SignedVote { winner: RwLock> } -/// Voting errors. -#[derive(Debug)] -pub enum VoteError { - /// Voter is not in the voters set. - UnauthorisedVoter -} - impl SignedVote { /// Create a new instance of BFT engine pub fn new(voters: HashSet
, threshold: usize) -> Self { @@ -71,7 +65,7 @@ impl SignedVote { fn can_vote(&self, bare_hash: &H256, signature: &Signature) -> Result<(), Error> { let signer = Address::from(try!(ec::recover(&signature, bare_hash)).sha3()); match self.voters.contains(&signer) { - false => try!(Err(VoteError::UnauthorisedVoter)), + false => try!(Err(EngineError::UnauthorisedVoter)), true => Ok(()), } } diff --git a/ethcore/src/engines/tendermint.rs b/ethcore/src/engines/tendermint.rs index 0a6747390..bc865ce2d 100644 --- a/ethcore/src/engines/tendermint.rs +++ b/ethcore/src/engines/tendermint.rs @@ -20,7 +20,7 @@ use common::*; use account_provider::AccountProvider; use block::*; use spec::CommonParams; -use engines::{Engine, ProposeCollect}; +use engines::{Engine, EngineError, ProposeCollect}; use evm::Schedule; use ethjson; @@ -89,22 +89,22 @@ impl Tendermint { p.validators.get(p.proposer_nonce%p.validator_n).unwrap().clone() } - fn propose_message(&self, message: UntrustedRlp) -> Option { + fn propose_message(&self, message: UntrustedRlp) -> Result { match *self.our_params.s.try_read().unwrap() { Step::Propose => (), - _ => return None, + _ => try!(Err(EngineError::WrongStep)), } - let proposal = message.val_at(0).unwrap_or_else(|| return None); + let proposal = try!(message.val_at(0)); let vote = ProposeCollect::new(proposal, self.our_params.validators.iter().cloned().collect(), self.threshold()); - let mut guard = self.our_params.s.try_write().unwrap(); + let mut guard = self.our_params.s.write(); *guard = Step::Prevote(vote); - Some(message.as_raw().to_vec()) + Ok(message.as_raw().to_vec()) } - fn prevote_message(&self, sender: Address, message: UntrustedRlp) -> Option { - None + fn prevote_message(&self, sender: Address, message: UntrustedRlp) -> Result { + try!(Err(EngineError::WrongStep)) } fn threshold(&self) -> usize { @@ -162,11 +162,11 @@ impl Engine for Tendermint { }) } - fn handle_message(&self, sender: Address, message: UntrustedRlp) -> Option { - match message.val_at(0).unwrap_or_else(|| return None) { + fn handle_message(&self, sender: Address, message: UntrustedRlp) -> Result { + match try!(message.val_at(0)) { 0u8 if sender == self.proposer() => self.propose_message(message), 1 => self.prevote_message(sender, message), - _ => None, + _ => try!(Err(EngineError::UnknownStep)), } } @@ -224,18 +224,18 @@ mod tests { use spec::Spec; /// Create a new test chain spec with `Tendermint` consensus engine. - fn new_test_authority() -> Spec { Spec::load(include_bytes!("../../res/bft.json")) } + fn new_test_tendermint() -> Spec { Spec::load(include_bytes!("../../res/tendermint.json")) } #[test] fn has_valid_metadata() { - let engine = new_test_authority().engine; + let engine = new_test_tendermint().engine; assert!(!engine.name().is_empty()); assert!(engine.version().major >= 1); } #[test] fn can_return_schedule() { - let engine = new_test_authority().engine; + let engine = new_test_tendermint().engine; let schedule = engine.schedule(&EnvInfo { number: 10000000, author: 0.into(), @@ -251,7 +251,7 @@ mod tests { #[test] fn can_do_seal_verification_fail() { - let engine = new_test_authority().engine; + let engine = new_test_tendermint().engine; let header: Header = Header::default(); let verify_result = engine.verify_block_basic(&header, None); @@ -265,7 +265,7 @@ mod tests { #[test] fn can_do_signature_verification_fail() { - let engine = new_test_authority().engine; + let engine = new_test_tendermint().engine; let mut header: Header = Header::default(); header.set_seal(vec![rlp::encode(&Signature::zero()).to_vec()]); @@ -284,7 +284,7 @@ mod tests { let addr = tap.insert_account("".sha3(), "").unwrap(); tap.unlock_account_permanently(addr, "".into()).unwrap(); - let spec = new_test_authority(); + let spec = new_test_tendermint(); let engine = &*spec.engine; let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); @@ -298,6 +298,15 @@ mod tests { assert!(b.try_seal(engine, seal).is_ok()); } + #[test] + fn propose_step(){ + let engine = new_test_tendermint().engine; + let tap = AccountProvider::transient_provider(); + let addr = tap.insert_account("1".sha3(), "1").unwrap(); + println!("{:?}", addr); + false; + } + #[test] fn handle_message() { false; diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index ccc926ce6..45ab99bcc 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -24,7 +24,7 @@ use client::Error as ClientError; use ipc::binary::{BinaryConvertError, BinaryConvertable}; use types::block_import_error::BlockImportError; use snapshot::Error as SnapshotError; -use engines::VoteError; +use engines::EngineError; pub use types::executed::{ExecutionError, CallError}; @@ -240,7 +240,7 @@ pub enum Error { /// Snapshot error. Snapshot(SnapshotError), /// Consensus vote error. - Vote(VoteError), + Engine(EngineError), } impl fmt::Display for Error { @@ -261,7 +261,7 @@ impl fmt::Display for Error { Error::StdIo(ref err) => err.fmt(f), Error::Snappy(ref err) => err.fmt(f), Error::Snapshot(ref err) => err.fmt(f), - Error::Vote(ref err) => + Error::Engine(ref err) => f.write_fmt(format_args!("Bad vote: {:?}", err)), } } @@ -366,10 +366,10 @@ impl From for Error { } } -impl From for Error { - fn from(err: VoteError) -> Error { +impl From for Error { + fn from(err: EngineError) -> Error { match err { - other => Error::Vote(other), + other => Error::Engine(other), } } }