Derive parent block information inside consensus engine

Author: remagpie

core/src/block.rs

@@ -220,25 +220,20 @@ impl<'x> OpenBlock<'x> {
     pub fn close(
         mut self,
         parent_header: &Header,
-        parent_common_params: &CommonParams,
         term_common_params: Option<&CommonParams>,
     ) -> Result<ClosedBlock, Error> {
         let unclosed_state = self.block.state.clone();
 
-        if let Err(e) =
-            self.engine.on_close_block(&mut self.block, parent_header, parent_common_params, term_common_params)
-        {
+        if let Err(e) = self.engine.on_close_block(&mut self.block, term_common_params) {
             warn!("Encountered error on closing the block: {}", e);
             return Err(e)
         }
         let header = self.block.header().clone();
         for handler in self.engine.action_handlers() {
-            handler.on_close_block(self.block.state_mut(), &header, parent_header, parent_common_params).map_err(
-                |e| {
-                    warn!("Encountered error in {}::on_close_block", handler.name());
-                    e
-                },
-            )?;
+            handler.on_close_block(self.block.state_mut(), &header).map_err(|e| {
+                warn!("Encountered error in {}::on_close_block", handler.name());
+                e
+            })?;
         }
 
         let state_root = self.block.state.commit().map_err(|e| {
@@ -262,23 +257,18 @@ impl<'x> OpenBlock<'x> {
     pub fn close_and_lock(
         mut self,
         parent_header: &Header,
-        parent_common_params: &CommonParams,
         term_common_params: Option<&CommonParams>,
     ) -> Result<LockedBlock, Error> {
-        if let Err(e) =
-            self.engine.on_close_block(&mut self.block, parent_header, parent_common_params, term_common_params)
-        {
+        if let Err(e) = self.engine.on_close_block(&mut self.block, term_common_params) {
             warn!("Encountered error on closing the block: {}", e);
             return Err(e)
         }
         let header = self.block.header().clone();
         for handler in self.engine.action_handlers() {
-            handler.on_close_block(self.block.state_mut(), &header, parent_header, parent_common_params).map_err(
-                |e| {
-                    warn!("Encountered error in {}::on_close_block", handler.name());
-                    e
-                },
-            )?;
+            handler.on_close_block(self.block.state_mut(), &header).map_err(|e| {
+                warn!("Encountered error in {}::on_close_block", handler.name());
+                e
+            })?;
         }
 
         let state_root = self.block.state.commit().map_err(|e| {
@@ -503,7 +493,6 @@ pub fn enact<C: ChainTimeInfo + EngineInfo + FindActionHandler + TermInfo>(
     b.populate_from(header);
     b.push_transactions(transactions, client, parent.number(), parent.timestamp())?;
 
-    let parent_common_params = client.common_params((*header.parent_hash()).into()).unwrap();
     let term_common_params = {
         let block_number = client
             .last_term_finished_block_num((*header.parent_hash()).into())
@@ -514,7 +503,7 @@ pub fn enact<C: ChainTimeInfo + EngineInfo + FindActionHandler + TermInfo>(
             Some(client.common_params((block_number).into()).expect("Common params should exist"))
         }
     };
-    b.close_and_lock(parent, &parent_common_params, term_common_params.as_ref())
+    b.close_and_lock(parent, term_common_params.as_ref())
 }
 
 #[cfg(test)]
@@ -532,9 +521,8 @@ mod tests {
         let genesis_header = scheme.genesis_header();
         let db = scheme.ensure_genesis_state(get_temp_state_db()).unwrap();
         let b = OpenBlock::try_new(&*scheme.engine, db, &genesis_header, Address::default(), vec![]).unwrap();
-        let parent_common_params = CommonParams::default_for_test();
         let term_common_params = CommonParams::default_for_test();
-        let b = b.close_and_lock(&genesis_header, &parent_common_params, Some(&term_common_params)).unwrap();
+        let b = b.close_and_lock(&genesis_header, Some(&term_common_params)).unwrap();
         let _ = b.seal(&*scheme.engine, vec![]);
     }
 }

core/src/client/test_client.rs

@@ -645,7 +645,7 @@ impl super::EngineClient for TestBlockChainClient {
 
 impl EngineInfo for TestBlockChainClient {
     fn common_params(&self, _block_id: BlockId) -> Option<CommonParams> {
-        unimplemented!()
+        Some(*self.scheme.engine.machine().genesis_common_params())
     }
 
     fn metadata_seq(&self, _block_id: BlockId) -> Option<u64> {

core/src/consensus/blake_pow/mod.rs

@@ -163,8 +163,6 @@ impl ConsensusEngine for BlakePoW {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        _parent_header: &Header,
-        _parent_common_params: &CommonParams,
         _term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
         let author = *block.header().author();

core/src/consensus/cuckoo/mod.rs

@@ -173,8 +173,6 @@ impl ConsensusEngine for Cuckoo {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        _parent_header: &Header,
-        _parent_common_params: &CommonParams,
         _term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
         let author = *block.header().author();
@@ -261,21 +259,13 @@ mod tests {
     #[test]
     fn on_close_block() {
         let scheme = Scheme::new_test_cuckoo();
-        let genesis_header = scheme.genesis_header();
         let engine = &*scheme.engine;
         let db = scheme.ensure_genesis_state(get_temp_state_db()).unwrap();
         let header = Header::default();
         let block = OpenBlock::try_new(engine, db, &header, Default::default(), vec![]).unwrap();
         let mut executed_block = block.block().clone();
 
-        assert!(engine
-            .on_close_block(
-                &mut executed_block,
-                &genesis_header,
-                &CommonParams::default_for_test(),
-                Some(&CommonParams::default_for_test())
-            )
-            .is_ok());
+        assert!(engine.on_close_block(&mut executed_block, Some(&CommonParams::default_for_test())).is_ok());
         assert_eq!(0xd, engine.machine().balance(&executed_block, header.author()).unwrap());
     }
 

core/src/consensus/mod.rs

@@ -225,8 +225,6 @@ pub trait ConsensusEngine: Sync + Send {
     fn on_close_block(
         &self,
         _block: &mut ExecutedBlock,
-        _parent_header: &Header,
-        _parent_common_params: &CommonParams,
         _term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
         Ok(())

core/src/consensus/null_engine/mod.rs

@@ -17,7 +17,7 @@
 mod params;
 
 use ckey::Address;
-use ctypes::{CommonParams, Header};
+use ctypes::CommonParams;
 
 use self::params::NullEngineParams;
 use super::ConsensusEngine;
@@ -58,8 +58,6 @@ impl ConsensusEngine for NullEngine {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        _parent_header: &Header,
-        _parent_common_params: &CommonParams,
         _term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
         let (author, total_reward) = {

core/src/consensus/simple_poa/mod.rs

@@ -121,8 +121,6 @@ impl ConsensusEngine for SimplePoA {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        _parent_header: &Header,
-        _parent_common_params: &CommonParams,
         _term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
         let author = *block.header().author();
@@ -186,9 +184,8 @@ mod tests {
         let db = scheme.ensure_genesis_state(get_temp_state_db()).unwrap();
         let genesis_header = scheme.genesis_header();
         let b = OpenBlock::try_new(engine, db, &genesis_header, Default::default(), vec![]).unwrap();
-        let parent_common_params = CommonParams::default_for_test();
         let term_common_params = CommonParams::default_for_test();
-        let b = b.close_and_lock(&genesis_header, &parent_common_params, Some(&term_common_params)).unwrap();
+        let b = b.close_and_lock(&genesis_header, Some(&term_common_params)).unwrap();
         if let Some(seal) = engine.generate_seal(Some(b.block()), &genesis_header).seal_fields() {
             assert!(b.try_seal(engine, seal).is_ok());
         }

core/src/consensus/solo/mod.rs

@@ -56,6 +56,10 @@ impl Solo {
             action_handlers,
         }
     }
+
+    fn client(&self) -> Option<Arc<dyn ConsensusClient>> {
+        self.client.read().as_ref()?.upgrade()
+    }
 }
 
 impl ConsensusEngine for Solo {
@@ -82,10 +86,13 @@ impl ConsensusEngine for Solo {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        parent_header: &Header,
-        parent_common_params: &CommonParams,
         _term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
+        let client = self.client().ok_or(EngineError::CannotOpenBlock)?;
+
+        let parent_hash = *block.header().parent_hash();
+        let parent = client.block_header(&parent_hash.into()).expect("Parent header must exist");
+        let parent_common_params = client.common_params(parent_hash.into()).expect("CommonParams of parent must exist");
         let author = *block.header().author();
         let (total_reward, total_min_fee) = {
             let transactions = block.transactions();
@@ -115,7 +122,7 @@ impl ConsensusEngine for Solo {
         let last_term_finished_block_num = {
             let header = block.header();
             let current_term_period = header.timestamp() / term_seconds;
-            let parent_term_period = parent_header.timestamp() / term_seconds;
+            let parent_term_period = parent.timestamp() / term_seconds;
             if current_term_period == parent_term_period {
                 return Ok(())
             }
@@ -173,9 +180,8 @@ mod tests {
         let db = client.scheme.ensure_genesis_state(get_temp_state_db()).unwrap();
         let genesis_header = client.scheme.genesis_header();
         let b = OpenBlock::try_new(&*engine, db, &genesis_header, Default::default(), vec![]).unwrap();
-        let parent_common_params = CommonParams::default_for_test();
         let term_common_params = CommonParams::default_for_test();
-        let b = b.close_and_lock(&genesis_header, &parent_common_params, Some(&term_common_params)).unwrap();
+        let b = b.close_and_lock(&genesis_header, Some(&term_common_params)).unwrap();
         if let Some(seal) = engine.generate_seal(Some(b.block()), &genesis_header).seal_fields() {
             assert!(b.try_seal(&*engine, seal).is_ok());
         }

core/src/consensus/stake/mod.rs

@@ -156,13 +156,7 @@ impl ActionHandler for Stake {
         action.verify(current_params, client, validators)
     }
 
-    fn on_close_block(
-        &self,
-        _state: &mut TopLevelState,
-        _header: &Header,
-        _parent_header: &Header,
-        _parent_common_params: &CommonParams,
-    ) -> StateResult<()> {
+    fn on_close_block(&self, _state: &mut TopLevelState, _header: &Header) -> StateResult<()> {
         Ok(())
     }
 }

core/src/consensus/tendermint/engine.rs

@@ -137,12 +137,16 @@ impl ConsensusEngine for Tendermint {
     fn on_close_block(
         &self,
         block: &mut ExecutedBlock,
-        parent_header: &Header,
-        parent_common_params: &CommonParams,
         term_common_params: Option<&CommonParams>,
     ) -> Result<(), Error> {
-        let block_number = block.header().number();
+        let client = self.client().ok_or(EngineError::CannotOpenBlock)?;
+
+        let parent_hash = *block.header().parent_hash();
+        let parent = client.block_header(&parent_hash.into()).expect("Parent header must exist").decode();
+        let parent_common_params = client.common_params(parent_hash.into()).expect("CommonParams of parent must exist");
         let author = *block.header().author();
+        let block_number = block.header().number();
+
         let (total_reward, total_min_fee) = {
             let transactions = block.transactions();
             let block_reward = self.block_reward(block_number);
@@ -178,7 +182,7 @@ impl ConsensusEngine for Tendermint {
             }
         }
 
-        if !is_term_changed(block.header(), parent_header, term_seconds) {
+        if !is_term_changed(block.header(), &parent, term_seconds) {
             return Ok(())
         }
 
@@ -187,7 +191,6 @@ impl ConsensusEngine for Tendermint {
             _ => {
                 let rewards = stake::drain_previous_rewards(block.state_mut())?;
                 let start_of_the_current_term = metadata.last_term_finished_block_num() + 1;
-                let client = self.client().ok_or(EngineError::CannotOpenBlock)?;
 
                 if term > 1 {
                     let start_of_the_previous_term = {