From df1cce8e7f649328b93d8d1f186bc902d8698a1a Mon Sep 17 00:00:00 2001 From: keorn Date: Thu, 1 Dec 2016 21:56:38 +0000 Subject: [PATCH] simplify seal verification --- ethcore/src/engines/tendermint/message.rs | 8 ++-- ethcore/src/engines/tendermint/mod.rs | 37 +++++++++++++------ .../src/engines/tendermint/vote_collector.rs | 2 +- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/ethcore/src/engines/tendermint/message.rs b/ethcore/src/engines/tendermint/message.rs index 06ee551ba..40d848679 100644 --- a/ethcore/src/engines/tendermint/message.rs +++ b/ethcore/src/engines/tendermint/message.rs @@ -85,6 +85,10 @@ impl ConsensusMessage { let public_key = try!(recover(&self.signature.into(), &block_info.as_raw().sha3())); Ok(public_to_address(&public_key)) } + + pub fn precommit_hash(&self) -> H256 { + message_info_rlp(self.height, self.round, Step::Precommit, self.block_hash).sha3() + } } impl PartialOrd for ConsensusMessage { @@ -170,10 +174,6 @@ pub fn message_info_rlp(height: Height, round: Round, step: Step, block_hash: Op s.out() } -pub fn message_info_rlp_from_header(header: &Header) -> Result { - let round = try!(consensus_round(header)); - Ok(message_info_rlp(header.number() as Height, round, Step::Precommit, Some(header.bare_hash()))) -} pub fn message_full_rlp(signature: &H520, vote_info: &Bytes) -> Bytes { let mut s = RlpStream::new_list(2); diff --git a/ethcore/src/engines/tendermint/mod.rs b/ethcore/src/engines/tendermint/mod.rs index 0cc8085d0..ae7d804d5 100644 --- a/ethcore/src/engines/tendermint/mod.rs +++ b/ethcore/src/engines/tendermint/mod.rs @@ -324,7 +324,7 @@ impl Engine for Tendermint { /// Get the address to be used as authority. fn on_new_block(&self, block: &mut ExecutedBlock) { - *self.authority.write() = *block.header().author() + *self.authority.write() = *block.header().author(); } /// Should this node participate. @@ -337,8 +337,11 @@ impl Engine for Tendermint { if let Some(ref ap) = *self.account_provider.lock() { let header = block.header(); let author = header.author(); - // Only proposer can generate seal. - if self.is_proposer(author).is_err() { return None; } + // Only proposer can generate seal if None was generated. + if self.is_proposer(author).is_err() && self.proposal.read().is_none() { + return None; + } + let height = header.number() as Height; let round = self.round.load(AtomicOrdering::SeqCst); let bh = Some(header.bare_hash()); @@ -421,6 +424,7 @@ impl Engine for Tendermint { } fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> { + // TODO: check total length of the last field let seal_length = header.seal().len(); if seal_length == self.seal_fields() { Ok(()) @@ -431,28 +435,37 @@ impl Engine for Tendermint { } } - /// Also transitions to Prevote if verifying Proposal. fn verify_block_unordered(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> { let proposal = try!(ConsensusMessage::new_proposal(header)); let proposer = try!(proposal.verify()); - try!(self.is_proposer(&proposer)); - self.votes.vote(proposal, proposer); - let block_info_hash = try!(message_info_rlp_from_header(header)).sha3(); - + if !self.is_authority(&proposer) { + try!(Err(EngineError::NotAuthorized(proposer))) + } + let precommit_hash = proposal.precommit_hash(); // TODO: use addresses recovered during precommit vote let mut signature_count = 0; + let mut origins = HashSet::new(); for rlp in UntrustedRlp::new(&header.seal()[2]).iter() { let signature: H520 = try!(rlp.as_val()); - let address = public_to_address(&try!(recover(&signature.into(), &block_info_hash))); + let address = public_to_address(&try!(recover(&signature.into(), &precommit_hash))); if !self.our_params.authorities.contains(&address) { try!(Err(EngineError::NotAuthorized(address))) } - signature_count += 1; + if origins.insert(address) { + signature_count += 1; + } else { + warn!(target: "poa", "verify_block_unordered: Duplicate signature from {} on the seal.", address) + } } - if signature_count > self.our_params.authority_n { - try!(Err(BlockError::InvalidSealArity(Mismatch { expected: self.our_params.authority_n, found: signature_count }))) + + // Check if its just a proposal if there is not enough precommits. + if !self.is_above_threshold(signature_count) { + try!(self.is_proposer(&proposer)); + *self.proposal.write() = proposal.block_hash.clone(); + self.votes.vote(proposal, proposer); } + Ok(()) } diff --git a/ethcore/src/engines/tendermint/vote_collector.rs b/ethcore/src/engines/tendermint/vote_collector.rs index 095b0fa37..93052a44a 100644 --- a/ethcore/src/engines/tendermint/vote_collector.rs +++ b/ethcore/src/engines/tendermint/vote_collector.rs @@ -120,7 +120,7 @@ impl VoteCollector { if origins.insert(origin) { n += 1; } else { - warn!("count_step_votes: authority {} has cast multiple step votes, this indicates malicious behaviour.", origin) + warn!("count_step_votes: Authority {} has cast multiple step votes, this indicates malicious behaviour.", origin) } } }