From 46fff8e4575740b99e30e37678396cd1e986e9d8 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Sun, 22 Sep 2019 05:12:25 +0900 Subject: [PATCH 1/2] Relplace message_info_rlp, message_hash with VoteOn::hash --- core/src/consensus/mod.rs | 3 +-- core/src/consensus/stake/actions.rs | 11 ++++++----- core/src/consensus/tendermint/message.rs | 25 +++++++----------------- core/src/consensus/tendermint/mod.rs | 24 ++++++++++++++--------- core/src/consensus/tendermint/worker.rs | 24 ++++++++++++++--------- 5 files changed, 44 insertions(+), 43 deletions(-) diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index bf92051d3b..513270d71a 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -31,8 +31,7 @@ pub use self::null_engine::NullEngine; pub use self::simple_poa::SimplePoA; pub use self::solo::Solo; pub use self::tendermint::{ - message_info_rlp, ConsensusMessage, Height, Step, Tendermint, TendermintParams, TimeGapParams, View, VoteOn, - VoteStep, + ConsensusMessage, Height, Step, Tendermint, TendermintParams, TimeGapParams, View, VoteOn, VoteStep, }; pub use self::validator_set::validator_list::RoundRobinValidator; pub use self::validator_set::{DynamicValidator, ValidatorSet}; diff --git a/core/src/consensus/stake/actions.rs b/core/src/consensus/stake/actions.rs index a41199c5ea..f96fd4d26d 100644 --- a/core/src/consensus/stake/actions.rs +++ b/core/src/consensus/stake/actions.rs @@ -358,10 +358,9 @@ impl Decodable for Action { #[cfg(test)] mod tests { use super::*; - use ccrypto::blake256; use ckey::sign_schnorr; use client::TestBlockChainClient; - use consensus::{message_info_rlp, ConsensusMessage, DynamicValidator, Step, VoteOn, VoteStep}; + use consensus::{ConsensusMessage, DynamicValidator, Step, VoteOn, VoteStep}; use rlp::rlp_encode_and_decode_test; #[test] @@ -418,12 +417,14 @@ mod tests { step: vote_step, block_hash, }; - let message_for_signature = - blake256(message_info_rlp(vote_step_twister(vote_step), block_hash_twister(block_hash))); + let twisted = VoteOn { + step: vote_step_twister(vote_step), + block_hash: block_hash_twister(block_hash), + }; let reversed_idx = client.get_validators().len() - 1 - signer_index; let pubkey = *client.get_validators().get(reversed_idx).unwrap().pubkey(); let privkey = *client.validator_keys.read().get(&pubkey).unwrap(); - let signature = sign_schnorr(&privkey, &message_for_signature).unwrap(); + let signature = sign_schnorr(&privkey, &twisted.hash()).unwrap(); ConsensusMessage { signature, diff --git a/core/src/consensus/tendermint/message.rs b/core/src/consensus/tendermint/message.rs index 8ed70543c5..a811e98da1 100644 --- a/core/src/consensus/tendermint/message.rs +++ b/core/src/consensus/tendermint/message.rs @@ -316,6 +316,12 @@ pub struct VoteOn { pub block_hash: Option, } +impl VoteOn { + pub fn hash(&self) -> H256 { + blake256(&self.rlp_bytes()) + } +} + /// Message transmitted between consensus participants. #[derive(Debug, PartialEq, Eq, Clone, Hash, Default, RlpDecodable, RlpEncodable)] pub struct ConsensusMessage { @@ -368,27 +374,10 @@ impl ConsensusMessage { } pub fn verify(&self, signer_public: &Public) -> Result { - let vote_info = message_info_rlp(self.on.step, self.on.block_hash); - verify_schnorr(signer_public, &self.signature, &blake256(vote_info)) + verify_schnorr(signer_public, &self.signature, &self.on.hash()) } } -pub fn message_info_rlp(step: VoteStep, block_hash: Option) -> Bytes { - let vote_on = VoteOn { - step, - block_hash, - }; - vote_on.rlp_bytes().into_vec() -} - -pub fn message_hash(step: VoteStep, block_hash: H256) -> H256 { - let vote_on = VoteOn { - step, - block_hash: Some(block_hash), - }; - blake256(&vote_on.rlp_bytes()) -} - #[cfg(test)] mod tests { use rlp::{self, rlp_encode_and_decode_test}; diff --git a/core/src/consensus/tendermint/mod.rs b/core/src/consensus/tendermint/mod.rs index 59d39c11f3..997e80cc78 100644 --- a/core/src/consensus/tendermint/mod.rs +++ b/core/src/consensus/tendermint/mod.rs @@ -35,7 +35,7 @@ use parking_lot::RwLock; use primitives::H256; use self::chain_notify::TendermintChainNotify; -pub use self::message::{message_info_rlp, ConsensusMessage, VoteOn, VoteStep}; +pub use self::message::{ConsensusMessage, VoteOn, VoteStep}; pub use self::params::{TendermintParams, TimeGapParams, TimeoutParams}; pub use self::types::{Height, Step, View}; use super::{stake, ValidatorSet}; @@ -129,7 +129,7 @@ mod tests { use primitives::Bytes; use super::super::BitSet; - use super::message::{message_info_rlp, VoteStep}; + use super::message::VoteStep; use crate::account_provider::AccountProvider; use crate::block::{ClosedBlock, OpenBlock}; use crate::client::TestBlockChainClient; @@ -231,8 +231,11 @@ mod tests { header.set_author(proposer); header.set_parent_hash(Default::default()); - let vote_info = message_info_rlp(VoteStep::new(3, 0, Step::Precommit), Some(*header.parent_hash())); - let signature2 = tap.get_account(&proposer, None).unwrap().sign_schnorr(&blake256(&vote_info)).unwrap(); + let vote_on = VoteOn { + step: VoteStep::new(3, 0, Step::Precommit), + block_hash: Some(*header.parent_hash()), + }; + let signature2 = tap.get_account(&proposer, None).unwrap().sign_schnorr(&vote_on.hash()).unwrap(); let seal = Seal::Tendermint { prev_view: 0, @@ -267,8 +270,11 @@ mod tests { header.set_author(proposer); header.set_parent_hash(block1_hash); - let vote_info = message_info_rlp(VoteStep::new(1, 0, Step::Precommit), Some(*header.parent_hash())); - let signature2 = tap.get_account(&proposer, None).unwrap().sign_schnorr(&blake256(&vote_info)).unwrap(); + let vote_info = VoteOn { + step: VoteStep::new(1, 0, Step::Precommit), + block_hash: Some(*header.parent_hash()), + }; + let signature2 = tap.get_account(&proposer, None).unwrap().sign_schnorr(&vote_info.hash()).unwrap(); let seal = Seal::Tendermint { prev_view: 0, @@ -287,9 +293,9 @@ mod tests { } let voter = validator3; - let signature3 = tap.get_account(&voter, None).unwrap().sign_schnorr(&blake256(&vote_info)).unwrap(); + let signature3 = tap.get_account(&voter, None).unwrap().sign_schnorr(&vote_info.hash()).unwrap(); let voter = validator0; - let signature0 = tap.get_account(&voter, None).unwrap().sign_schnorr(&blake256(&vote_info)).unwrap(); + let signature0 = tap.get_account(&voter, None).unwrap().sign_schnorr(&vote_info.hash()).unwrap(); let seal = Seal::Tendermint { prev_view: 0, @@ -304,7 +310,7 @@ mod tests { assert!(engine.verify_block_external(&header).is_ok()); let bad_voter = insert_and_unlock(&tap, "101"); - let bad_signature = tap.get_account(&bad_voter, None).unwrap().sign_schnorr(&blake256(vote_info)).unwrap(); + let bad_signature = tap.get_account(&bad_voter, None).unwrap().sign_schnorr(&vote_info.hash()).unwrap(); let seal = Seal::Tendermint { prev_view: 0, diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index ea6c94a361..0b459d68a9 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -1122,9 +1122,11 @@ impl Worker { debug_assert_eq!(Ok(self.view), TendermintSealView::new(header.seal()).consensus_view()); - let vote_step = VoteStep::new(header.number() as Height, self.view, Step::Propose); - let vote_info = message_info_rlp(vote_step, Some(hash)); - let signature = self.sign(blake256(&vote_info)).expect("I am proposer"); + let vote_on = VoteOn { + step: VoteStep::new(header.number() as Height, self.view, Step::Propose), + block_hash: Some(hash), + }; + let signature = self.sign(vote_on.hash()).expect("I am proposer"); self.votes.vote( ConsensusMessage::new_proposal(signature, &*self.validators, header, self.view, prev_proposer_idx) .expect("I am proposer"), @@ -1193,8 +1195,10 @@ impl Worker { } let previous_block_view = TendermintSealView::new(header.seal()).previous_block_view()?; - let step = VoteStep::new(header.number() - 1, previous_block_view, Step::Precommit); - let precommit_hash = message_hash(step, *header.parent_hash()); + let precommit_vote_on = VoteOn { + step: VoteStep::new(header.number() - 1, previous_block_view, Step::Precommit), + block_hash: Some(*header.parent_hash()), + }; let mut voted_validators = BitSet::new(); let grand_parent_hash = self @@ -1204,7 +1208,7 @@ impl Worker { .parent_hash(); for (bitset_index, signature) in seal_view.signatures()? { let public = self.validators.get(&grand_parent_hash, bitset_index); - if !verify_schnorr(&public, &signature, &precommit_hash)? { + if !verify_schnorr(&public, &signature, &precommit_vote_on.hash())? { let address = public_to_address(&public); return Err(EngineError::BlockNotAuthorized(address.to_owned()).into()) } @@ -1476,11 +1480,13 @@ impl Worker { fn repropose_block(&mut self, block: encoded::Block) { let header = block.decode_header(); - let vote_step = VoteStep::new(header.number() as Height, self.view, Step::Propose); - let vote_info = message_info_rlp(vote_step, Some(header.hash())); + let vote_on = VoteOn { + step: VoteStep::new(header.number() as Height, self.view, Step::Propose), + block_hash: Some(header.hash()), + }; let parent_hash = header.parent_hash(); let prev_proposer_idx = self.block_proposer_idx(*parent_hash).expect("Prev block must exists"); - let signature = self.sign(blake256(&vote_info)).expect("I am proposer"); + let signature = self.sign(vote_on.hash()).expect("I am proposer"); self.votes.vote( ConsensusMessage::new_proposal(signature, &*self.validators, &header, self.view, prev_proposer_idx) .expect("I am proposer"), From 91ee242e8650da4ec52617706062201fe5d9a078 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Sun, 22 Sep 2019 05:19:38 +0900 Subject: [PATCH 2/2] Make sign function hash argument by itself --- core/src/consensus/tendermint/worker.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index 0b459d68a9..46f5afc33b 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -20,7 +20,6 @@ use std::sync::{Arc, Weak}; use std::thread::{Builder, JoinHandle}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use ccrypto::blake256; use ckey::{public_to_address, verify_schnorr, Address, SchnorrSignature}; use cnetwork::{EventSender, NodeId}; use crossbeam_channel as crossbeam; @@ -838,13 +837,12 @@ impl Worker { step: VoteStep::new(height, r, self.step.to_step()), block_hash, }; - let vote_info = on.rlp_bytes(); let signer_index = self.signer_index().or_else(|| { ctrace!(ENGINE, "No message, since there is no engine signer."); None })?; let signature = self - .sign(blake256(&vote_info)) + .sign(&on) .map_err(|error| { ctrace!(ENGINE, "{}th validator could not sign the message {}", signer_index, error); error @@ -1126,7 +1124,7 @@ impl Worker { step: VoteStep::new(header.number() as Height, self.view, Step::Propose), block_hash: Some(hash), }; - let signature = self.sign(vote_on.hash()).expect("I am proposer"); + let signature = self.sign(&vote_on).expect("I am proposer"); self.votes.vote( ConsensusMessage::new_proposal(signature, &*self.validators, header, self.view, prev_proposer_idx) .expect("I am proposer"), @@ -1486,7 +1484,7 @@ impl Worker { }; let parent_hash = header.parent_hash(); let prev_proposer_idx = self.block_proposer_idx(*parent_hash).expect("Prev block must exists"); - let signature = self.sign(vote_on.hash()).expect("I am proposer"); + let signature = self.sign(&vote_on).expect("I am proposer"); self.votes.vote( ConsensusMessage::new_proposal(signature, &*self.validators, &header, self.view, prev_proposer_idx) .expect("I am proposer"), @@ -1519,8 +1517,8 @@ impl Worker { self.signer.set_to_keep_decrypted_account(ap, address); } - fn sign(&self, hash: H256) -> Result { - self.signer.sign(hash).map_err(Into::into) + fn sign(&self, vote_on: &VoteOn) -> Result { + self.signer.sign(vote_on.hash()).map_err(Into::into) } fn signer_index(&self) -> Option {