From bb18193250cfdef3162d2c855e8dfd9342a7420c Mon Sep 17 00:00:00 2001 From: Juhyung Park Date: Tue, 22 Oct 2019 16:05:00 +0900 Subject: [PATCH 1/5] Add get_version and set_version When the data structure of data that is saved in the KVDB is changed, CodeChain should migrate the data. To identify the stored data structure, we need to keep a version number. This commit adds code to save and load a version number. To make the DB easier to manage, this code saves versions in the same column with the same prefix. --- core/src/db_version.rs | 40 ++++++++++++++++++++++++++++++++++++++++ core/src/lib.rs | 1 + 2 files changed, 41 insertions(+) create mode 100644 core/src/db_version.rs diff --git a/core/src/db_version.rs b/core/src/db_version.rs new file mode 100644 index 0000000000..dc46a95203 --- /dev/null +++ b/core/src/db_version.rs @@ -0,0 +1,40 @@ +// Copyright 2019 Kodebox, Inc. +// This file is part of CodeChain. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . +use kvdb::{DBTransaction, KeyValueDB}; + +use crate::db; + +pub const VERSION_KEY_PREFIX: &[u8] = b"version_"; +/// Save the version of Tendermint backup where the key below is pointing +pub const VERSION_KEY_TENDERMINT_BACKUP: &[u8] = b"version_tendermint-backup"; + +/// To support data values that are saved before the version scheme return 0 if the version does not exist +pub fn get_version(db: &dyn KeyValueDB, key: &[u8]) -> u32 { + let value = db.get(db::COL_EXTRA, key).expect("Low level database error. Some issue with disk?"); + if let Some(bytes) = value { + rlp::decode(&bytes) + } else { + 0 + } +} + +pub fn set_version(batch: &mut DBTransaction, key: &[u8], value: u32) { + assert!( + key.starts_with(VERSION_KEY_PREFIX), + "Version keys should be prefixed with".to_owned() + std::str::from_utf8(VERSION_KEY_PREFIX).unwrap() + ); + batch.put(db::COL_EXTRA, key, &rlp::encode(&value).into_vec()); +} diff --git a/core/src/lib.rs b/core/src/lib.rs index 8c28839bdd..26cf864d8f 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -67,6 +67,7 @@ mod client; mod codechain_machine; mod consensus; mod db; +mod db_version; pub mod encoded; mod error; mod invoice; From 410ab7851eae7e41772d62c07898c79f250003c3 Mon Sep 17 00:00:00 2001 From: Juhyung Park Date: Tue, 22 Oct 2019 15:15:52 +0900 Subject: [PATCH 2/5] Save finalized_view_of_previous_block and finalized_view_of_current_block Tendermint worker's last_confirmed_view is used for two purposes. One is the previous height's finalized view, and the other one is the current height's finalized view. This commit is part of splitting the last_confirmed_view variable. --- core/src/consensus/tendermint/backup.rs | 138 +++++++++++++++++++++--- core/src/consensus/tendermint/worker.rs | 10 +- 2 files changed, 129 insertions(+), 19 deletions(-) diff --git a/core/src/consensus/tendermint/backup.rs b/core/src/consensus/tendermint/backup.rs index bc22a664f1..1f8c45b5c7 100644 --- a/core/src/consensus/tendermint/backup.rs +++ b/core/src/consensus/tendermint/backup.rs @@ -20,18 +20,21 @@ use primitives::H256; use super::message::ConsensusMessage; use super::types::{Height, Step, View}; use crate::db; +use crate::db_version; const BACKUP_KEY: &[u8] = b"tendermint-backup"; +const BACKUP_VERSION: u32 = 1; pub struct BackupView<'a> { pub height: &'a Height, pub view: &'a View, pub step: &'a Step, pub votes: &'a [ConsensusMessage], - pub last_confirmed_view: &'a View, + pub finalized_view_of_previous_block: &'a View, + pub finalized_view_of_current_block: &'a Option, } -pub struct BackupData { +pub struct BackupDataV0 { pub height: Height, pub view: View, pub step: Step, @@ -40,25 +43,111 @@ pub struct BackupData { pub last_confirmed_view: View, } +pub struct BackupDataV1 { + pub height: Height, + pub view: View, + pub step: Step, + pub votes: Vec, + pub proposal: Option, + pub finalized_view_of_previous_block: View, + pub finalized_view_of_current_block: Option, +} + pub fn backup(db: &dyn KeyValueDB, backup_data: BackupView) { let BackupView { height, view, step, votes, - last_confirmed_view, + finalized_view_of_previous_block, + finalized_view_of_current_block, } = backup_data; let mut s = rlp::RlpStream::new(); - s.begin_list(5); + s.begin_list(6); s.append(height).append(view).append(step).append_list(votes); - s.append(last_confirmed_view); + s.append(finalized_view_of_previous_block); + s.append(finalized_view_of_current_block); let mut batch = DBTransaction::new(); + debug_assert!( + db_version::VERSION_KEY_TENDERMINT_BACKUP.ends_with(BACKUP_KEY), + "version key should end with the backup key" + ); + db_version::set_version(&mut batch, db_version::VERSION_KEY_TENDERMINT_BACKUP, BACKUP_VERSION); batch.put(db::COL_EXTRA, BACKUP_KEY, &s.drain().into_vec()); db.write(batch).expect("Low level database error. Some issue with disk?"); } -pub fn restore(db: &dyn KeyValueDB) -> Option { +pub fn restore(db: &dyn KeyValueDB) -> Option { + let version = db_version::get_version(db, db_version::VERSION_KEY_TENDERMINT_BACKUP); + if version < BACKUP_VERSION { + migrate(db); + } + load_v1(db) +} + +fn find_proposal(votes: &[ConsensusMessage], height: Height, view: View) -> Option { + votes + .iter() + .rev() + .map(|vote| &vote.on) + .find(|vote_on| { + vote_on.step.step == Step::Propose && vote_on.step.view == view && vote_on.step.height == height + }) + .map(|vote_on| vote_on.block_hash) + .unwrap_or(None) +} + +fn migrate(db: &dyn KeyValueDB) { + let version = db_version::get_version(db, db_version::VERSION_KEY_TENDERMINT_BACKUP); + assert!( + version < BACKUP_VERSION, + "migrate function should be called when the saved version is less than BACKUP_VERSION" + ); + + match version { + 0 => { + migrate_from_0_to_1(db); + } + _ => panic!("Invalid migration version {}", version), + } +} + +fn migrate_from_0_to_1(db: &dyn KeyValueDB) { + let v0 = if let Some(v0) = load_v0(db) { + v0 + } else { + return + }; + let step = v0.step; + let v1 = BackupDataV1 { + height: v0.height, + view: v0.view, + step: v0.step, + votes: v0.votes, + proposal: v0.proposal, + // This is not a correct behavior if step == Step::Commit. + // In Commit state, the Tendermint module overwrote the last_confirmed_view to finalized_view_of_current_block. + // So we can't restore finalized_view_of_previous block. + // The code below maintain older code's behavior: + finalized_view_of_previous_block: v0.last_confirmed_view, + finalized_view_of_current_block: if step == Step::Commit { + Some(v0.last_confirmed_view) + } else { + None + }, + }; + backup(db, BackupView { + height: &v1.height, + view: &v1.view, + step: &v1.step, + votes: &v1.votes, + finalized_view_of_previous_block: &v1.finalized_view_of_previous_block, + finalized_view_of_current_block: &v1.finalized_view_of_current_block, + }) +} + +fn load_v0(db: &dyn KeyValueDB) -> Option { let value = db.get(db::COL_EXTRA, BACKUP_KEY).expect("Low level database error. Some issue with disk?"); let (height, view, step, votes, last_confirmed_view) = value.map(|bytes| { let bytes = bytes.into_vec(); @@ -68,7 +157,7 @@ pub fn restore(db: &dyn KeyValueDB) -> Option { let proposal = find_proposal(&votes, height, view); - Some(BackupData { + Some(BackupDataV0 { height, view, step, @@ -78,14 +167,29 @@ pub fn restore(db: &dyn KeyValueDB) -> Option { }) } -fn find_proposal(votes: &[ConsensusMessage], height: Height, view: View) -> Option { - votes - .iter() - .rev() - .map(|vote| &vote.on) - .find(|vote_on| { - vote_on.step.step == Step::Propose && vote_on.step.view == view && vote_on.step.height == height - }) - .map(|vote_on| vote_on.block_hash) - .unwrap_or(None) +fn load_v1(db: &dyn KeyValueDB) -> Option { + #[derive(RlpDecodable)] + struct Backup { + height: Height, + view: View, + step: Step, + votes: Vec, + finalized_view_of_previous_block: View, + finalized_view_of_current_block: Option, + } + + let value = db.get(db::COL_EXTRA, BACKUP_KEY).expect("Low level database error. Some issue with disk?")?; + let backup: Backup = rlp::decode(&value); + + let proposal = find_proposal(&backup.votes, backup.height, backup.view); + + Some(BackupDataV1 { + height: backup.height, + view: backup.view, + step: backup.step, + votes: backup.votes, + proposal, + finalized_view_of_previous_block: backup.finalized_view_of_previous_block, + finalized_view_of_current_block: backup.finalized_view_of_current_block, + }) } diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index 95f9b3d3a3..0c7b1f4b0d 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -981,7 +981,8 @@ impl Worker { view: &self.view, step: &self.step.to_step(), votes: &self.votes.get_all(), - last_confirmed_view: &self.last_confirmed_view, + finalized_view_of_previous_block: &self.last_confirmed_view, + finalized_view_of_current_block: &Some(self.last_confirmed_view), }); } @@ -1001,7 +1002,12 @@ impl Worker { self.step = backup_step; self.height = backup.height; self.view = backup.view; - self.last_confirmed_view = backup.last_confirmed_view; + if backup.step == Step::Commit { + self.last_confirmed_view = + backup.finalized_view_of_current_block.expect("In commit step the finalized view exist") + } else { + self.last_confirmed_view = backup.finalized_view_of_previous_block; + } if let Some(proposal) = backup.proposal { if client.block(&BlockId::Hash(proposal)).is_some() { self.proposal = Proposal::ProposalImported(proposal); From 0a36abecd11a874a3bfe5fde9e660edcc35abdbd Mon Sep 17 00:00:00 2001 From: Juhyung Park Date: Tue, 22 Oct 2019 15:42:37 +0900 Subject: [PATCH 3/5] Split last_confirmed_view variable Tendermint worker's last_confirmed_view is used for two purposes. One is the previous height's finalized view, and the other one is the current height's finalized view. This commit is part of splitting the last_confirmed_view variable. --- core/src/consensus/tendermint/worker.rs | 90 ++++++++++++++++--------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index 0c7b1f4b0d..1a8002be92 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -85,8 +85,11 @@ struct Worker { last_two_thirds_majority: TwoThirdsMajority, /// hash of the proposed block, used for seal submission. proposal: Proposal, - /// The last confirmed view from the commit step. - last_confirmed_view: View, + /// The finalized view of the previous height's block. + /// The signatures for the previous block is signed for the view below. + finalized_view_of_previous_block: View, + /// The finalized view of the current height's block. + finalized_view_of_current_block: Option, /// Set used to determine the current validators. validators: Arc, /// Channel to the network extension, must be set later. @@ -187,7 +190,8 @@ impl Worker { signer: Default::default(), last_two_thirds_majority: TwoThirdsMajority::Empty, proposal: Proposal::None, - last_confirmed_view: 0, + finalized_view_of_previous_block: 0, + finalized_view_of_current_block: None, validators, extension, votes_received: MutTrigger::new(BitSet::new()), @@ -418,7 +422,11 @@ impl Worker { return false } - let vote_step = VoteStep::new(self.height, self.last_confirmed_view, Step::Precommit); + let vote_step = VoteStep::new( + self.height, + self.finalized_view_of_current_block.expect("finalized_view_of_current_height is not None in Commit state"), + Step::Precommit, + ); if self.step.is_commit_timedout() && self.check_current_block_exists() { cinfo!(ENGINE, "Transition to Propose because best block is changed after commit timeout"); return true @@ -597,8 +605,10 @@ impl Worker { self.client().update_sealing(BlockId::Hash(parent_block_hash), true); } - fn save_last_confirmed_view(&mut self, view: View) { - self.last_confirmed_view = view; + /// Do we need this function? + fn set_finalized_view_in_current_height(&mut self, view: View) { + assert_eq!(self.finalized_view_of_current_block, None); + self.finalized_view_of_current_block = Some(view); } fn increment_view(&mut self, n: View) { @@ -608,7 +618,28 @@ impl Worker { self.votes_received = MutTrigger::new(BitSet::new()); } - fn move_to_height(&mut self, height: Height) { + /// Move to the next height. + fn move_to_the_next_height(&mut self) { + assert!( + self.step.is_commit(), + "move_to_the_next_height should be called in Commit state, but the current step is {:?}", + self.step + ); + cinfo!(ENGINE, "Transitioning to height {}.", self.height + 1); + self.last_two_thirds_majority = TwoThirdsMajority::Empty; + self.height += 1; + self.view = 0; + self.proposal = Proposal::None; + self.votes_received = MutTrigger::new(BitSet::new()); + self.finalized_view_of_previous_block = + self.finalized_view_of_current_block.expect("self.step == Step::Commit"); + self.finalized_view_of_current_block = None; + } + + /// Jump to the height. + /// This function is called when new blocks are received from block sync. + /// This function could be called at any state. + fn jump_to_height(&mut self, height: Height, finalized_view_of_previous_height: View) { assert!(height > self.height, "{} < {}", height, self.height); cinfo!(ENGINE, "Transitioning to height {}.", height); self.last_two_thirds_majority = TwoThirdsMajority::Empty; @@ -616,6 +647,8 @@ impl Worker { self.view = 0; self.proposal = Proposal::None; self.votes_received = MutTrigger::new(BitSet::new()); + self.finalized_view_of_previous_block = finalized_view_of_previous_height; + self.finalized_view_of_current_block = None; } #[allow(clippy::cognitive_complexity)] @@ -736,7 +769,7 @@ impl Worker { Step::Commit => { cinfo!(ENGINE, "move_to_step: Commit."); let (view, block_hash) = state.committed().expect("commit always has committed_view"); - self.save_last_confirmed_view(view); + self.set_finalized_view_in_current_height(view); let proposal_received = self.is_proposal_received(self.height, view, block_hash); let proposal_imported = self.client().block(&block_hash.into()).is_some(); @@ -850,8 +883,7 @@ impl Worker { && has_enough_aligned_votes { if self.can_move_from_commit_to_propose() { - let height = self.height; - self.move_to_height(height + 1); + self.move_to_the_next_height(); self.move_to_step(TendermintState::Propose, is_restoring); return } @@ -957,9 +989,11 @@ impl Worker { _ => {} }; } else if current_height < height { - self.move_to_height(height); - let prev_block_view = TendermintSealView::new(proposal.seal()).previous_block_view().unwrap(); - self.save_last_confirmed_view(prev_block_view); + let finalized_view_of_previous_height = + TendermintSealView::new(proposal.seal()).previous_block_view().unwrap(); + + self.jump_to_height(height, finalized_view_of_previous_height); + let proposal_is_for_view0 = self.votes.has_votes_for( &VoteStep { height, @@ -981,8 +1015,8 @@ impl Worker { view: &self.view, step: &self.step.to_step(), votes: &self.votes.get_all(), - finalized_view_of_previous_block: &self.last_confirmed_view, - finalized_view_of_current_block: &Some(self.last_confirmed_view), + finalized_view_of_previous_block: &self.finalized_view_of_previous_block, + finalized_view_of_current_block: &self.finalized_view_of_current_block, }); } @@ -1002,12 +1036,9 @@ impl Worker { self.step = backup_step; self.height = backup.height; self.view = backup.view; - if backup.step == Step::Commit { - self.last_confirmed_view = - backup.finalized_view_of_current_block.expect("In commit step the finalized view exist") - } else { - self.last_confirmed_view = backup.finalized_view_of_previous_block; - } + self.finalized_view_of_previous_block = backup.finalized_view_of_previous_block; + self.finalized_view_of_current_block = backup.finalized_view_of_current_block; + if let Some(proposal) = backup.proposal { if client.block(&BlockId::Hash(proposal)).is_some() { self.proposal = Proposal::ProposalImported(proposal); @@ -1045,7 +1076,7 @@ impl Worker { let view = self.view; - let last_block_view = &self.last_confirmed_view; + let last_block_view = &self.finalized_view_of_previous_block; assert_eq!(self.prev_block_hash(), parent_hash); let (precommits, precommit_indices) = self @@ -1305,8 +1336,7 @@ impl Worker { return } - let height = self.height; - self.move_to_height(height + 1); + self.move_to_the_next_height(); TendermintState::Propose } TendermintState::CommitTimedout { @@ -1374,7 +1404,7 @@ impl Worker { // the previous step. So, act as the last precommit step. VoteStep { height: self.height, - view: self.last_confirmed_view, + view: self.finalized_view_of_current_block.expect("self.step == Step::Commit"), step: Step::Precommit, } } else { @@ -1585,8 +1615,7 @@ impl Worker { if enacted.first() == Some(&committed_block_hash) { cdebug!(ENGINE, "Committed block {} is now the best block", committed_block_hash); if self.can_move_from_commit_to_propose() { - let new_height = self.height + 1; - self.move_to_height(new_height); + self.move_to_the_next_height(); self.move_to_step(TendermintState::Propose, false); return } @@ -1612,11 +1641,10 @@ impl Worker { let full_header = header.decode(); if self.height < header.number() { cinfo!(ENGINE, "Received a commit: {:?}.", header.number()); - let prev_block_view = TendermintSealView::new(full_header.seal()) + let finalized_view_of_previous_height = TendermintSealView::new(full_header.seal()) .previous_block_view() .expect("Imported block already checked"); - self.move_to_height(header.number()); - self.save_last_confirmed_view(prev_block_view); + self.jump_to_height(header.number(), finalized_view_of_previous_height); } } if height_at_begin != self.height { @@ -1809,7 +1837,7 @@ impl Worker { // the previous step. So, act as the last precommit step. VoteStep { height: self.height, - view: self.last_confirmed_view, + view: self.finalized_view_of_current_block.expect("self.step == Step::Commit"), step: Step::Precommit, } } else { From cd8f9574279d4fc3d72f2b359330e5785cf037e4 Mon Sep 17 00:00:00 2001 From: Juhyung Park Date: Fri, 18 Oct 2019 16:44:48 +0900 Subject: [PATCH 4/5] Rename views to author_view and finalized_view author_view: view when the block was created. finalized_view: view when the block was finalized. --- core/src/consensus/tendermint/types.rs | 8 +++- core/src/consensus/tendermint/worker.rs | 50 ++++++++++++------------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/core/src/consensus/tendermint/types.rs b/core/src/consensus/tendermint/types.rs index 7cf582ebfe..373c9e7d33 100644 --- a/core/src/consensus/tendermint/types.rs +++ b/core/src/consensus/tendermint/types.rs @@ -224,13 +224,17 @@ impl<'a> TendermintSealView<'a> { } } - pub fn previous_block_view(&self) -> Result { + /// The parent block is finalized at this view. + /// Signatures in the seal field is signed for this view. + pub fn parent_block_finalized_view(&self) -> Result { let view_rlp = self.seal.get(0).expect("block went through verify_block_basic; block has .seal_fields() fields; qed"); UntrustedRlp::new(view_rlp.as_slice()).as_val() } - pub fn consensus_view(&self) -> Result { + /// Block is created at auth_view. + /// Block verifier use other_view to verify the author + pub fn author_view(&self) -> Result { let view_rlp = self.seal.get(1).expect("block went through verify_block_basic; block has .seal_fields() fields; qed"); UntrustedRlp::new(view_rlp.as_slice()).as_val() diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index 1a8002be92..a29a6fb552 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -928,9 +928,9 @@ impl Worker { let height = proposal.number() as Height; let seal_view = TendermintSealView::new(proposal.seal()); - let prev_block_view = seal_view.previous_block_view().expect("The proposal is verified"); + let parent_block_finalized_view = seal_view.parent_block_finalized_view().expect("The proposal is verified"); let on = VoteOn { - step: VoteStep::new(height - 1, prev_block_view, Step::Precommit), + step: VoteStep::new(height - 1, parent_block_finalized_view, Step::Precommit), block_hash: Some(*proposal.parent_hash()), }; for (index, signature) in seal_view.signatures().expect("The proposal is verified") { @@ -990,7 +990,7 @@ impl Worker { }; } else if current_height < height { let finalized_view_of_previous_height = - TendermintSealView::new(proposal.seal()).previous_block_view().unwrap(); + TendermintSealView::new(proposal.seal()).parent_block_finalized_view().unwrap(); self.jump_to_height(height, finalized_view_of_previous_height); @@ -1095,16 +1095,15 @@ impl Worker { fn proposal_generated(&mut self, sealed_block: &SealedBlock) { let proposal_height = sealed_block.header().number(); let proposal_seal = sealed_block.header().seal(); - let proposal_view = TendermintSealView::new(proposal_seal) - .consensus_view() - .expect("Generated proposal should have a valid seal"); + let proposal_author_view = + TendermintSealView::new(proposal_seal).author_view().expect("Generated proposal should have a valid seal"); assert!(proposal_height <= self.height, "A proposal cannot be generated on the future height"); - if proposal_height < self.height || (proposal_height == self.height && proposal_view != self.view) { + if proposal_height < self.height || (proposal_height == self.height && proposal_author_view != self.view) { ctrace!( ENGINE, "Proposal is generated on the height {} and view {}. Current height is {} and view is {}", proposal_height, - proposal_view, + proposal_author_view, self.height, self.view, ); @@ -1131,7 +1130,7 @@ impl Worker { ); return } - debug_assert_eq!(Ok(self.view), TendermintSealView::new(header.seal()).consensus_view()); + debug_assert_eq!(Ok(self.view), TendermintSealView::new(header.seal()).author_view()); self.vote_on_header_for_proposal(&header).expect("I'm a proposer"); @@ -1152,8 +1151,8 @@ impl Worker { } let height = header.number(); - let view = TendermintSealView::new(header.seal()).consensus_view().unwrap(); - let score = calculate_score(height, view); + let author_view = TendermintSealView::new(header.seal()).author_view().unwrap(); + let score = calculate_score(height, author_view); if *header.score() != score { return Err(BlockError::InvalidScore(Mismatch { @@ -1168,13 +1167,13 @@ impl Worker { fn verify_block_external(&self, header: &Header) -> Result<(), Error> { let height = header.number() as usize; - let view = TendermintSealView::new(header.seal()).consensus_view()?; - ctrace!(ENGINE, "Verify external at {}-{}, {:?}", height, view, header); + let author_view = TendermintSealView::new(header.seal()).author_view()?; + ctrace!(ENGINE, "Verify external at {}-{}, {:?}", height, author_view, header); let proposer = header.author(); if !self.is_authority(header.parent_hash(), proposer) { return Err(EngineError::BlockNotAuthorized(*proposer).into()) } - self.check_view_proposer(header.parent_hash(), header.number(), view, &proposer)?; + self.check_view_proposer(header.parent_hash(), header.number(), author_view, &proposer)?; let seal_view = TendermintSealView::new(header.seal()); let bitset_count = seal_view.bitset()?.count(); let precommits_count = seal_view.precommits().item_count()?; @@ -1197,9 +1196,9 @@ impl Worker { return Err(BlockError::InvalidSeal.into()) } - let previous_block_view = TendermintSealView::new(header.seal()).previous_block_view()?; + let parent_block_finalized_view = TendermintSealView::new(header.seal()).parent_block_finalized_view()?; let precommit_vote_on = VoteOn { - step: VoteStep::new(header.number() - 1, previous_block_view, Step::Precommit), + step: VoteStep::new(header.number() - 1, parent_block_finalized_view, Step::Precommit), block_hash: Some(*header.parent_hash()), }; @@ -1642,7 +1641,7 @@ impl Worker { if self.height < header.number() { cinfo!(ENGINE, "Received a commit: {:?}.", header.number()); let finalized_view_of_previous_height = TendermintSealView::new(full_header.seal()) - .previous_block_view() + .parent_block_finalized_view() .expect("Imported block already checked"); self.jump_to_height(header.number(), finalized_view_of_previous_height); } @@ -1791,15 +1790,14 @@ impl Worker { // The proposer re-proposed its locked proposal. // If we already imported the proposal, we should set `proposal` here. if c.block(&BlockId::Hash(header_view.hash())).is_some() { - let generated_view = TendermintSealView::new(header_view.seal()) - .consensus_view() - .expect("Imported block is verified"); + let author_view = + TendermintSealView::new(header_view.seal()).author_view().expect("Imported block is verified"); cdebug!( ENGINE, "Received a proposal({}) by a locked proposer. current view: {}, original proposal's view: {}", header_view.hash(), proposed_view, - generated_view + author_view ); self.proposal = Proposal::new_imported(header_view.hash()); } else { @@ -1975,13 +1973,14 @@ impl Worker { let block = self.client().block(&height.into()).expect("Parent block should exist"); let block_hash = block.hash(); let seal = block.seal(); - let view = TendermintSealView::new(&seal).consensus_view().expect("Block is already verified and imported"); + let author_view = + TendermintSealView::new(&seal).author_view().expect("Block is already verified and imported"); let votes = self .votes .get_all_votes_in_round(&VoteStep { height, - view, + view: author_view, step: Step::Precommit, }) .into_iter() @@ -1994,9 +1993,10 @@ impl Worker { let child_block = self.client().block(&(height + 1).into()).expect("Parent block should exist"); let child_block_header_seal = child_block.header().seal(); let child_block_seal_view = TendermintSealView::new(&child_block_header_seal); - let view = child_block_seal_view.previous_block_view().expect("Verified block"); + let parent_block_finalized_view = + child_block_seal_view.parent_block_finalized_view().expect("Verified block"); let on = VoteOn { - step: VoteStep::new(height, view, Step::Precommit), + step: VoteStep::new(height, parent_block_finalized_view, Step::Precommit), block_hash: Some(block.hash()), }; let mut votes = Vec::new(); From 6b7e18223e88a5eb4a7f00491f702d8a2470bc8e Mon Sep 17 00:00:00 2001 From: Juhyung Park Date: Fri, 18 Oct 2019 16:57:34 +0900 Subject: [PATCH 5/5] Use correct view when creating a Commit message --- core/src/consensus/tendermint/worker.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index a29a6fb552..eed20ebe84 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -1972,15 +1972,13 @@ impl Worker { if height == self.height - 1 { let block = self.client().block(&height.into()).expect("Parent block should exist"); let block_hash = block.hash(); - let seal = block.seal(); - let author_view = - TendermintSealView::new(&seal).author_view().expect("Block is already verified and imported"); + let finalized_view = self.finalized_view_of_previous_block; let votes = self .votes .get_all_votes_in_round(&VoteStep { height, - view: author_view, + view: finalized_view, step: Step::Precommit, }) .into_iter()