Fix the condition to check whether the new block could change the best block

When a new block is imported in Tendermint consensus, CodeChain checks
the conditions below to judge whether it is safe to change the best
block:

First, whether the score is higher than before. Second, whether the
height of a new block is higher than the finalized block's
height.

This commit fixes the code that is calculating the finalized
block's height erroneously.

Author: majecty

core/src/blockchain/blockchain.rs

@@ -184,17 +184,19 @@ impl BlockChain {
         let new_header = new_block.header_view();
         let parent_hash_of_new_block = new_header.parent_hash();
         let parent_details_of_new_block = self.block_details(&parent_hash_of_new_block).expect("Invalid parent hash");
+        let prev_best_proposal_hash = self.best_proposal_block_hash();
+        let prev_best_hash = self.best_block_hash();
 
         if parent_details_of_new_block.total_score + new_header.score() > self.best_proposal_block_detail().total_score
-            && engine.can_change_canon_chain(&new_header)
+            && engine.can_change_canon_chain(&new_header, prev_best_hash, prev_best_proposal_hash)
         {
             cinfo!(
                 BLOCKCHAIN,
                 "Block #{}({}) has higher total score, changing the best proposal/canonical chain.",
                 new_header.number(),
                 new_header.hash()
             );
-            let prev_best_hash = self.best_block_hash();
+
             let route = tree_route(self, prev_best_hash, parent_hash_of_new_block)
                 .expect("blocks being imported always within recent history; qed");
 

core/src/blockchain/headerchain.rs

@@ -242,9 +242,11 @@ impl HeaderChain {
     fn best_header_changed(&self, new_header: &HeaderView, engine: &dyn CodeChainEngine) -> BestHeaderChanged {
         let parent_hash_of_new_header = new_header.parent_hash();
         let parent_details_of_new_header = self.block_details(&parent_hash_of_new_header).expect("Invalid parent hash");
+        let prev_best_proposal_hash = self.best_proposal_header_hash();
+        let prev_best_hash = self.best_header_hash();
         let is_new_best = parent_details_of_new_header.total_score + new_header.score()
             > self.best_proposal_header_detail().total_score
-            && engine.can_change_canon_chain(&new_header);
+            && engine.can_change_canon_chain(&new_header, prev_best_hash, prev_best_proposal_hash);
 
         if is_new_best {
             ctrace!(
@@ -256,7 +258,6 @@ impl HeaderChain {
             // on new best block we need to make sure that all ancestors
             // are moved to "canon chain"
             // find the route between old best block and the new one
-            let prev_best_hash = self.best_header_hash();
             let route = tree_route(self, prev_best_hash, parent_hash_of_new_header)
                 .expect("blocks being imported always within recent history; qed");
 

core/src/consensus/mod.rs

@@ -266,10 +266,15 @@ pub trait ConsensusEngine: Sync + Send {
         header.hash()
     }
 
-    /// In PoW consensus, the higher scored block became the best block.
+    /// In PoW consensus, the higher scored block becomes the best block.
     /// In Tendermint consensus, the highest scored block may not be the best block.
-    /// Only the child of the current best block could be the next best block in Tendermint consensus.
-    fn can_change_canon_chain(&self, _header: &HeaderView) -> bool {
+    /// Only the descendant of the current best block could be the next best block in Tendermint consensus.
+    fn can_change_canon_chain(
+        &self,
+        _new_header: &HeaderView,
+        _previous_best_hash: H256,
+        _previous_best_proposal_hash: H256,
+    ) -> bool {
         true
     }
 

core/src/consensus/tendermint/engine.rs

@@ -308,15 +308,14 @@ impl ConsensusEngine for Tendermint {
         header.parent_hash()
     }
 
-    fn can_change_canon_chain(&self, header: &HeaderView) -> bool {
-        let (result, receiver) = crossbeam::bounded(1);
-        self.inner
-            .send(worker::Event::AllowedHeight {
-                result,
-            })
-            .unwrap();
-        let allowed_height = receiver.recv().unwrap();
-        header.number() >= allowed_height
+
+    fn can_change_canon_chain(
+        &self,
+        new_header: &HeaderView,
+        prev_best_hash: H256,
+        prev_best_proposal_hash: H256,
+    ) -> bool {
+        new_header.parent_hash() == prev_best_hash || new_header.parent_hash() == prev_best_proposal_hash
     }
 
     fn action_handlers(&self) -> &[Arc<dyn ActionHandler>] {

core/src/consensus/tendermint/worker.rs

@@ -133,9 +133,6 @@ pub enum Event {
         ap: Arc<AccountProvider>,
         address: Address,
     },
-    AllowedHeight {
-        result: crossbeam::Sender<Height>,
-    },
     Restore(crossbeam::Sender<()>),
     ProposalBlock {
         signature: SchnorrSignature,
@@ -307,16 +304,6 @@ impl Worker {
                             }) => {
                                 inner.set_signer(ap, address);
                             }
-                            Ok(Event::AllowedHeight {
-                                result,
-                            }) => {
-                                let allowed_height = if inner.step.is_commit() {
-                                    inner.height + 1
-                                } else {
-                                    inner.height
-                                };
-                                result.send(allowed_height).unwrap();
-                            }
                             Ok(Event::Restore(result)) => {
                                 inner.restore();
                                 result.send(()).unwrap();