Fix the view calculation bug

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<View>,
 }
 
-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<ConsensusMessage>,
+    pub proposal: Option<H256>,
+    pub finalized_view_of_previous_block: View,
+    pub finalized_view_of_current_block: Option<View>,
+}
+
 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<BackupData> {
+pub fn restore(db: &dyn KeyValueDB) -> Option<BackupDataV1> {
+    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<H256> {
+    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<BackupDataV0> {
     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<BackupData> {
 
     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<BackupData> {
     })
 }
 
-fn find_proposal(votes: &[ConsensusMessage], height: Height, view: View) -> Option<H256> {
-    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<BackupDataV1> {
+    #[derive(RlpDecodable)]
+    struct Backup {
+        height: Height,
+        view: View,
+        step: Step,
+        votes: Vec<ConsensusMessage>,
+        finalized_view_of_previous_block: View,
+        finalized_view_of_current_block: Option<View>,
+    }
+
+    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,
+    })
 }

core/src/consensus/tendermint/types.rs

@@ -224,13 +224,17 @@ impl<'a> TendermintSealView<'a> {
         }
     }
 
-    pub fn previous_block_view(&self) -> Result<u64, DecoderError> {
+    /// 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<u64, DecoderError> {
         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<u64, DecoderError> {
+    /// Block is created at auth_view.
+    /// Block verifier use other_view to verify the author
+    pub fn author_view(&self) -> Result<u64, DecoderError> {
         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()