Update the term id and the last block id when the term changes

Seulgi Kim · mergify[bot] · commit 83e8a4dba391 · 2019-05-24T06:18:35.000Z
diff --git a/core/src/codechain_machine.rs b/core/src/codechain_machine.rs
@@ -250,6 +250,16 @@ impl CodeChainMachine {
         live.state_mut().add_balance(address, amount).map_err(StateError::from)?;
         Ok(())
     }
+
+    pub fn change_term_id(
+        &self,
+        live: &mut ExecutedBlock,
+        last_term_finished_block_num: u64,
+        current_term_id: u64,
+    ) -> Result<(), Error> {
+        live.state_mut().change_term_id(last_term_finished_block_num, current_term_id)?;
+        Ok(())
+    }
 }
 
 fn is_order_disabled() -> bool {
diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs
@@ -81,7 +81,7 @@ impl ConsensusEngine for Solo<CodeChainMachine> {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        _parent_header: &Header,
+        parent_header: &Header,
         parent_common_params: &CommonParams,
     ) -> Result<(), Error> {
         let author = *block.header().author();
@@ -102,6 +102,20 @@ impl ConsensusEngine for Solo<CodeChainMachine> {
         let stakeholders_share = stake::stakeholders_share(total_min_fee, &stakes);
         self.machine.add_balance(block, &author, total_reward - stakeholders_share)?;
 
+        let term_seconds = parent_common_params.term_seconds();
+        if term_seconds == 0 {
+            return Ok(())
+        }
+        let (last_term_finished_block_num, current_term_id) = {
+            let header = block.header();
+            let term_id = header.timestamp() / term_seconds;
+            let parent_term_id = parent_header.timestamp() / term_seconds;
+            if term_id == parent_term_id {
+                return Ok(())
+            }
+            (header.number(), term_id)
+        };
+        self.machine.change_term_id(block, last_term_finished_block_num, current_term_id)?;
         Ok(())
     }
 
diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs
@@ -131,7 +131,7 @@ impl ConsensusEngine for Tendermint {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        _parent_header: &Header,
+        parent_header: &Header,
         parent_common_params: &CommonParams,
     ) -> Result<(), Error> {
         let author = *block.header().author();
@@ -152,6 +152,20 @@ impl ConsensusEngine for Tendermint {
         let stakeholders_share = stake::stakeholders_share(total_min_fee, &stakes);
         self.machine.add_balance(block, &author, total_reward - stakeholders_share)?;
 
+        let term_seconds = parent_common_params.term_seconds();
+        if term_seconds == 0 {
+            return Ok(())
+        }
+        let (last_term_finished_block_num, current_term_id) = {
+            let header = block.header();
+            let term_id = header.timestamp() / term_seconds;
+            let parent_term_id = parent_header.timestamp() / term_seconds;
+            if term_id == parent_term_id {
+                return Ok(())
+            }
+            (header.number(), term_id)
+        };
+        self.machine.change_term_id(block, last_term_finished_block_num, current_term_id)?;
         Ok(())
     }
 
diff --git a/spec/Dynamic-Validator.md b/spec/Dynamic-Validator.md
@@ -222,7 +222,7 @@ candidates = [ [address, deposits, nominate_end_at]+ ], address asc
 pending_rewards = [ [withdraw_at, address, quantity]+ ], [withdraw_at, address] asc
 banned = [ address+ ], address asc
 jailed = [ [address, deposits, custody_until, kicked_at]+ ], address asc
-term_id = [ block number, term id ]
+term_id = [ the last block number of the previous term, the current term id ]
 ```
 
 ### on TermEnd events
diff --git a/state/src/impls/top_level.rs b/state/src/impls/top_level.rs
@@ -974,6 +974,12 @@ impl TopState for TopLevelState {
         Ok(())
     }
 
+    fn change_term_id(&mut self, last_term_finished_block_num: u64, current_term_id: u64) -> StateResult<()> {
+        let mut metadata = self.get_metadata_mut()?;
+        metadata.change_term(last_term_finished_block_num, current_term_id);
+        Ok(())
+    }
+
     fn update_action_data(&mut self, key: &H256, data: Bytes) -> StateResult<()> {
         let mut action_data = self.get_action_data_mut(key)?;
         *action_data = data.into();
diff --git a/state/src/item/metadata.rs b/state/src/item/metadata.rs
@@ -21,11 +21,18 @@ use ctypes::{CommonParams, ShardId};
 
 use crate::CacheableItem;
 
+#[derive(Clone, Debug, Default, PartialEq)]
+struct TermMetadata {
+    last_term_finished_block_num: u64,
+    current_term_id: u64,
+}
+
 #[derive(Clone, Debug, PartialEq)]
 pub struct Metadata {
     number_of_shards: ShardId,
     number_of_initial_shards: ShardId,
     hashes: Vec<H256>,
+    term: TermMetadata,
     seq: usize,
     params: Option<CommonParams>,
 }
@@ -36,6 +43,7 @@ impl Metadata {
             number_of_shards,
             number_of_initial_shards: number_of_shards,
             hashes: vec![],
+            term: Default::default(),
             seq: 0,
             params: None,
         }
@@ -73,6 +81,13 @@ impl Metadata {
     pub fn params(&self) -> Option<&CommonParams> {
         self.params.as_ref()
     }
+
+    pub fn change_term(&mut self, last_term_finished_block_num: u64, current_term_id: u64) {
+        assert!(self.term.last_term_finished_block_num < last_term_finished_block_num);
+        assert!(self.term.current_term_id < current_term_id);
+        self.term.last_term_finished_block_num = last_term_finished_block_num;
+        self.term.current_term_id = current_term_id;
+    }
 }
 
 impl Default for Metadata {
@@ -93,28 +108,62 @@ const PREFIX: u8 = super::METADATA_PREFIX;
 
 impl Encodable for Metadata {
     fn rlp_append(&self, s: &mut RlpStream) {
+        const INITIAL_LEN: usize = 4;
+        const TERM_LEN: usize = 2;
+        const PARAMS_LEN: usize = 2;
+        let mut len = INITIAL_LEN;
+
+        let term_changed = self.term != Default::default();
+        if term_changed {
+            len += TERM_LEN;
+        }
+
         let params_changed = self.seq != 0;
         if params_changed {
-            s.begin_list(6);
-        } else {
-            s.begin_list(4);
+            if !term_changed {
+                len += TERM_LEN;
+            }
+            len += PARAMS_LEN;
         }
-        s.append(&PREFIX)
+        s.begin_list(len)
+            .append(&PREFIX)
             .append(&self.number_of_shards)
             .append(&self.number_of_initial_shards)
             .append_list(&self.hashes);
+        if term_changed {
+            s.append(&self.term.last_term_finished_block_num).append(&self.term.current_term_id);
+        }
         if params_changed {
-            s.append(&self.seq);
-            s.append(self.params.as_ref().unwrap());
+            if !term_changed {
+                const DEFAULT_LAST_TERM_FINISHED_BLOCK_NUM: u64 = 0;
+                const DEFAULT_CURRENT_TERM_ID: u64 = 0;
+                s.append(&DEFAULT_LAST_TERM_FINISHED_BLOCK_NUM).append(&DEFAULT_CURRENT_TERM_ID);
+            }
+            s.append(&self.seq).append(self.params.as_ref().unwrap());
         }
     }
 }
 
 impl Decodable for Metadata {
     fn decode(rlp: &UntrustedRlp) -> Result<Self, DecoderError> {
-        let (seq, params) = match rlp.item_count()? {
-            4 => (0, None),
-            6 => (rlp.val_at(4)?, Some(rlp.val_at(5)?)),
+        let (term, seq, params) = match rlp.item_count()? {
+            4 => (TermMetadata::default(), 0, None),
+            6 => (
+                TermMetadata {
+                    last_term_finished_block_num: rlp.val_at(4)?,
+                    current_term_id: rlp.val_at(5)?,
+                },
+                0,
+                None,
+            ),
+            8 => (
+                TermMetadata {
+                    last_term_finished_block_num: rlp.val_at(4)?,
+                    current_term_id: rlp.val_at(5)?,
+                },
+                rlp.val_at(6)?,
+                Some(rlp.val_at(7)?),
+            ),
             item_count => {
                 return Err(DecoderError::RlpInvalidLength {
                     got: item_count,
@@ -131,6 +180,7 @@ impl Decodable for Metadata {
             number_of_shards: rlp.val_at(1)?,
             number_of_initial_shards: rlp.val_at(2)?,
             hashes: rlp.list_at(3)?,
+            term,
             seq,
             params,
         })
@@ -190,11 +240,59 @@ mod tests {
     }
 
     #[test]
-    fn metadata_with_seq() {
+    fn check_backward_compatibility() {
+        let metadata = Metadata {
+            number_of_shards: 10,
+            number_of_initial_shards: 1,
+            hashes: vec![],
+            term: Default::default(),
+            seq: 0,
+            params: None,
+        };
+        let mut rlp = RlpStream::new_list(4);
+        rlp.append(&PREFIX).append(&10u16).append(&1u16).append_list::<H256, H256>(&[]);
+        assert_eq!(metadata.rlp_bytes(), rlp.drain());
+    }
+
+    #[test]
+    fn metadata_without_term_with_seq() {
+        let metadata = Metadata {
+            number_of_shards: 10,
+            number_of_initial_shards: 1,
+            hashes: vec![],
+            term: Default::default(),
+            seq: 3,
+            params: Some(CommonParams::default_for_test()),
+        };
+        rlp_encode_and_decode_test!(metadata);
+    }
+
+    #[test]
+    fn metadata_with_term_without_seq() {
+        let metadata = Metadata {
+            number_of_shards: 10,
+            number_of_initial_shards: 1,
+            hashes: vec![],
+            term: TermMetadata {
+                last_term_finished_block_num: 1,
+                current_term_id: 100,
+            },
+            seq: 0,
+            params: None,
+        };
+        rlp_encode_and_decode_test!(metadata);
+    }
+
+    #[test]
+    fn metadata_with_term_and_seq() {
         let metadata = Metadata {
             number_of_shards: 10,
             number_of_initial_shards: 1,
             hashes: vec![],
+            term: TermMetadata {
+                last_term_finished_block_num: 1,
+                current_term_id: 100,
+            },
             seq: 3,
             params: Some(CommonParams::default_for_test()),
         };
diff --git a/state/src/traits.rs b/state/src/traits.rs
@@ -177,6 +177,8 @@ pub trait TopState {
     fn store_text(&mut self, key: &H256, text: Text, sig: &Signature) -> StateResult<()>;
     fn remove_text(&mut self, key: &H256, sig: &Signature) -> StateResult<()>;
 
+    fn change_term_id(&mut self, last_term_finished_block_num: u64, current_term_id: u64) -> StateResult<()>;
+
     fn update_action_data(&mut self, key: &H256, data: Bytes) -> StateResult<()>;
     fn remove_action_data(&mut self, key: &H256);
 }

Original file line number	Diff line number	Diff line change
`@@ -177,6 +177,8 @@ pub trait TopState {`
`177`	`177`	`fn store_text(&mut self, key: &H256, text: Text, sig: &Signature) -> StateResult<()>;`
`178`	`178`	`fn remove_text(&mut self, key: &H256, sig: &Signature) -> StateResult<()>;`
`179`	`179`
	`180`	`+ fn change_term_id(&mut self, last_term_finished_block_num: u64, current_term_id: u64) -> StateResult<()>;`
	`181`	`+`
`180`	`182`	`fn update_action_data(&mut self, key: &H256, data: Bytes) -> StateResult<()>;`
`181`	`183`	`fn remove_action_data(&mut self, key: &H256);`
`182`	`184`	`}`