From 9edc479a9caab53a5145ed12547c6549957c5a75 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 17:43:42 +0900 Subject: [PATCH 1/3] Include ConsensusClient in Solo engine --- core/src/consensus/solo/mod.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index 6faf5dc0d5..1a50b18920 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -16,22 +16,25 @@ mod params; -use std::sync::Arc; +use std::sync::{Arc, Weak}; use ckey::Address; use cstate::{ActionHandler, HitHandler}; use ctypes::{CommonParams, Header}; +use parking_lot::RwLock; use self::params::SoloParams; use super::stake; use super::{ConsensusEngine, Seal}; use crate::block::{ExecutedBlock, IsBlock}; +use crate::client::ConsensusClient; use crate::codechain_machine::CodeChainMachine; use crate::consensus::{EngineError, EngineType}; use crate::error::Error; /// A consensus engine which does not provide any consensus mechanism. pub struct Solo { + client: RwLock>>, params: SoloParams, machine: CodeChainMachine, action_handlers: Vec>, @@ -47,6 +50,7 @@ impl Solo { action_handlers.push(Arc::new(stake::Stake::new(params.genesis_stakes.clone()))); Solo { + client: Default::default(), params, machine, action_handlers, @@ -127,6 +131,10 @@ impl ConsensusEngine for Solo { Ok(()) } + fn register_client(&self, client: Weak) { + *self.client.write() = Some(Weak::clone(&client)); + } + fn block_reward(&self, _block_number: u64) -> u64 { self.params.block_reward } @@ -146,25 +154,30 @@ impl ConsensusEngine for Solo { #[cfg(test)] mod tests { + use std::sync::Arc; + use ctypes::{CommonParams, Header}; use primitives::H520; use crate::block::{IsBlock, OpenBlock}; + use crate::client::{ConsensusClient, TestBlockChainClient}; use crate::scheme::Scheme; use crate::tests::helpers::get_temp_state_db; #[test] fn seal() { let scheme = Scheme::new_test_solo(); - let engine = &*scheme.engine; - 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 client = Arc::new(TestBlockChainClient::new_with_scheme(scheme)); + let engine = client.scheme.engine.clone(); + engine.register_client(Arc::downgrade(&(client.clone() as Arc))); + 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(); if let Some(seal) = engine.generate_seal(Some(b.block()), &genesis_header).seal_fields() { - assert!(b.try_seal(engine, seal).is_ok()); + assert!(b.try_seal(&*engine, seal).is_ok()); } } From 16bbc905b744ab11ce460d2e663e341577eb398c Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 18:17:08 +0900 Subject: [PATCH 2/3] Add a helper function for obtaining client --- core/src/consensus/tendermint/engine.rs | 18 +++--------------- core/src/consensus/tendermint/mod.rs | 4 ++++ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 4cb410751a..b6ff3dcdea 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -187,13 +187,7 @@ 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 - .read() - .as_ref() - .ok_or(EngineError::CannotOpenBlock)? - .upgrade() - .ok_or(EngineError::CannotOpenBlock)?; + let client = self.client().ok_or(EngineError::CannotOpenBlock)?; if term > 1 { let start_of_the_previous_term = { @@ -273,7 +267,7 @@ impl ConsensusEngine for Tendermint { let inner = self.inner.clone(); let extension = service.register_extension(move |api| TendermintExtension::new(inner, timeouts, api)); - let client = Weak::clone(self.client.read().as_ref().unwrap()); + let client = Arc::downgrade(&self.client().unwrap()); self.extension_initializer.send((extension, client)).unwrap(); let (result, receiver) = crossbeam::bounded(1); @@ -317,13 +311,7 @@ impl ConsensusEngine for Tendermint { } fn possible_authors(&self, block_number: Option) -> Result>, EngineError> { - let client = self - .client - .read() - .as_ref() - .ok_or(EngineError::CannotOpenBlock)? - .upgrade() - .ok_or(EngineError::CannotOpenBlock)?; + let client = self.client().ok_or(EngineError::CannotOpenBlock)?; let block_hash = match block_number { None => { client.block_header(&BlockId::Latest).expect("latest block must exist").hash() // the latest block diff --git a/core/src/consensus/tendermint/mod.rs b/core/src/consensus/tendermint/mod.rs index 85edcd6a9a..73f1c5fc2a 100644 --- a/core/src/consensus/tendermint/mod.rs +++ b/core/src/consensus/tendermint/mod.rs @@ -115,6 +115,10 @@ impl Tendermint { has_signer: false.into(), }) } + + fn client(&self) -> Option> { + self.client.read().as_ref()?.upgrade() + } } const SEAL_FIELDS: usize = 4; From 80670355b5d24fbf492fdb97e7b83c9e2db6abc2 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 18:35:17 +0900 Subject: [PATCH 3/3] Derive parent block information inside consensus engine --- core/src/block.rs | 36 +++++++++---------------- core/src/client/test_client.rs | 2 +- core/src/consensus/blake_pow/mod.rs | 2 -- core/src/consensus/cuckoo/mod.rs | 12 +-------- core/src/consensus/mod.rs | 2 -- core/src/consensus/null_engine/mod.rs | 4 +-- core/src/consensus/simple_poa/mod.rs | 5 +--- core/src/consensus/solo/mod.rs | 16 +++++++---- core/src/consensus/stake/mod.rs | 8 +----- core/src/consensus/tendermint/engine.rs | 13 +++++---- core/src/consensus/tendermint/mod.rs | 3 +-- core/src/miner/miner.rs | 3 +-- core/src/miner/sealing_queue.rs | 3 +-- state/src/action_handler/hit.rs | 8 +----- state/src/action_handler/mod.rs | 8 +----- 15 files changed, 41 insertions(+), 84 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index 103d5903a7..7d5a5dea68 100644 --- a/core/src/block.rs +++ b/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 { 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 { - 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( 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( 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![]); } } diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 0a279626e7..31775914f8 100644 --- a/core/src/client/test_client.rs +++ b/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 { - unimplemented!() + Some(*self.scheme.engine.machine().genesis_common_params()) } fn metadata_seq(&self, _block_id: BlockId) -> Option { diff --git a/core/src/consensus/blake_pow/mod.rs b/core/src/consensus/blake_pow/mod.rs index 8e0b20a6e3..010fa35ae6 100644 --- a/core/src/consensus/blake_pow/mod.rs +++ b/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(); diff --git a/core/src/consensus/cuckoo/mod.rs b/core/src/consensus/cuckoo/mod.rs index ad088f2454..d904120d67 100644 --- a/core/src/consensus/cuckoo/mod.rs +++ b/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()); } diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index 6841b859d4..c0d78b1485 100644 --- a/core/src/consensus/mod.rs +++ b/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(()) diff --git a/core/src/consensus/null_engine/mod.rs b/core/src/consensus/null_engine/mod.rs index 832f6165b7..26f40199d1 100644 --- a/core/src/consensus/null_engine/mod.rs +++ b/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) = { diff --git a/core/src/consensus/simple_poa/mod.rs b/core/src/consensus/simple_poa/mod.rs index 848af86b70..a6b72120b8 100644 --- a/core/src/consensus/simple_poa/mod.rs +++ b/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()); } diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index 1a50b18920..414e45a00f 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -56,6 +56,10 @@ impl Solo { action_handlers, } } + + fn client(&self) -> Option> { + 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()); } diff --git a/core/src/consensus/stake/mod.rs b/core/src/consensus/stake/mod.rs index cf0a07022a..02789499fe 100644 --- a/core/src/consensus/stake/mod.rs +++ b/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(()) } } diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index b6ff3dcdea..3ea3282c7a 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/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 = { diff --git a/core/src/consensus/tendermint/mod.rs b/core/src/consensus/tendermint/mod.rs index 73f1c5fc2a..721ce1cb34 100644 --- a/core/src/consensus/tendermint/mod.rs +++ b/core/src/consensus/tendermint/mod.rs @@ -161,9 +161,8 @@ mod tests { let genesis_header = scheme.genesis_header(); let b = OpenBlock::try_new(scheme.engine.as_ref(), db, &genesis_header, proposer, vec![]).unwrap(); let seal = scheme.engine.generate_seal(None, &genesis_header).seal_fields().unwrap(); - let common_params = CommonParams::default_for_test(); let term_common_params = CommonParams::default_for_test(); - let b = b.close(&genesis_header, &common_params, Some(&term_common_params)).unwrap(); + let b = b.close(&genesis_header, Some(&term_common_params)).unwrap(); (b, seal) } diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 8c7c446262..6dc64bb987 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -603,7 +603,6 @@ impl Miner { let parent_header = chain.block_header(&parent_hash.into()).expect("Parent header MUST exist"); (parent_header.decode(), parent_hash) }; - let parent_common_params = chain.common_params(parent_hash.into()).unwrap(); let term_common_params = { let block_number = chain .last_term_finished_block_num(parent_hash.into()) @@ -614,7 +613,7 @@ impl Miner { Some(chain.common_params((block_number).into()).expect("Common params should exist")) } }; - let block = open_block.close(&parent_header, &parent_common_params, term_common_params.as_ref())?; + let block = open_block.close(&parent_header, term_common_params.as_ref())?; let fetch_seq = |p: &Public| { let address = public_to_address(p); diff --git a/core/src/miner/sealing_queue.rs b/core/src/miner/sealing_queue.rs index 3866274d04..16f282367e 100644 --- a/core/src/miner/sealing_queue.rs +++ b/core/src/miner/sealing_queue.rs @@ -87,9 +87,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, vec![]).unwrap(); - let common_params = CommonParams::default_for_test(); let term_common_params = CommonParams::default_for_test(); - b.close(&genesis_header, &common_params, Some(&term_common_params)).unwrap() + b.close(&genesis_header, Some(&term_common_params)).unwrap() } #[test] diff --git a/state/src/action_handler/hit.rs b/state/src/action_handler/hit.rs index 407524fe1f..7891b99e36 100644 --- a/state/src/action_handler/hit.rs +++ b/state/src/action_handler/hit.rs @@ -86,13 +86,7 @@ impl ActionHandler for HitHandler { Ok(()) } - 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<()> { let address = self.close_count(); let action_data = state.action_data(&address)?.unwrap_or_default(); let prev_counter: u32 = rlp::decode(&*action_data).unwrap(); diff --git a/state/src/action_handler/mod.rs b/state/src/action_handler/mod.rs index 519f8e62df..dd7c1da1ac 100644 --- a/state/src/action_handler/mod.rs +++ b/state/src/action_handler/mod.rs @@ -47,13 +47,7 @@ pub trait ActionHandler: Send + Sync { Ok(some_action_data) } - 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<()>; } pub trait FindActionHandler {