From 36ce74e1637e0674d44151e236cd2fab35706081 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Mon, 25 Nov 2019 21:03:09 +0900 Subject: [PATCH 01/20] Remove redundant variable in tendermint/engine --- core/src/consensus/tendermint/engine.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 3261e087a5..6039bd647c 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -173,8 +173,7 @@ impl ConsensusEngine for Tendermint { return Ok(()) } - let block_author = *block.header().author(); - stake::update_validator_weights(&mut block.state_mut(), &block_author)?; + stake::update_validator_weights(&mut block.state_mut(), &author)?; stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; From cbfc7bc6b6351e36741aa36994a837d3b2a3e44a Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Mon, 25 Nov 2019 21:05:59 +0900 Subject: [PATCH 02/20] Change block_number_if_term_changed to is_term_changed `block_number_if_term_changed` was returning the current block number if the term changed, which could be calculated in the outer scope. --- core/src/consensus/tendermint/engine.rs | 37 +++++++++---------------- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 6039bd647c..188e554899 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -25,7 +25,7 @@ use ckey::{public_to_address, Address}; use cnetwork::NetworkService; use crossbeam_channel as crossbeam; use cstate::{ActionHandler, TopStateView}; -use ctypes::{BlockHash, BlockNumber, CommonParams, Header}; +use ctypes::{BlockHash, CommonParams, Header}; use num_rational::Ratio; use super::super::stake; @@ -141,10 +141,11 @@ impl ConsensusEngine for Tendermint { parent_common_params: &CommonParams, term_common_params: Option<&CommonParams>, ) -> Result<(), Error> { + let block_number = block.header().number(); let author = *block.header().author(); let (total_reward, total_min_fee) = { let transactions = block.transactions(); - let block_reward = self.block_reward(block.header().number()); + let block_reward = self.block_reward(block_number); let total_min_fee: u64 = transactions.iter().map(|tx| tx.fee).sum(); let min_fee = transactions.iter().map(|tx| CodeChainMachine::min_cost(&parent_common_params, &tx.action)).sum(); @@ -164,9 +165,7 @@ impl ConsensusEngine for Tendermint { if metadata.current_term_id() == 0 { self.machine.add_balance(block, &author, block_author_reward)?; - if let Some(block_number) = - block_number_if_term_changed(block.header(), parent_header, parent_common_params) - { + if is_term_changed(block.header(), parent_header, parent_common_params) { // First term change stake::on_term_close(block.state_mut(), block_number, &[])?; } @@ -178,13 +177,9 @@ impl ConsensusEngine for Tendermint { stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; let term_common_params = term_common_params.expect("TermCommonParams should exist"); - let last_term_finished_block_num = if let Some(block_number) = - block_number_if_term_changed(block.header(), parent_header, term_common_params) - { - block_number - } else { + if !is_term_changed(block.header(), parent_header, term_common_params) { return Ok(()) - }; + } let rewards = stake::drain_previous_rewards(&mut block.state_mut())?; let start_of_the_current_term = metadata.last_term_finished_block_num() + 1; @@ -213,7 +208,7 @@ impl ConsensusEngine for Tendermint { }; let banned = stake::Banned::load_from_state(block.state())?; - let start_of_the_current_term_header = if block.header().number() == start_of_the_current_term { + let start_of_the_current_term_header = if block_number == start_of_the_current_term { encoded::Header::new(block.header().clone().rlp_bytes().to_vec()) } else { client.block_header(&start_of_the_current_term.into()).unwrap() @@ -241,7 +236,7 @@ impl ConsensusEngine for Tendermint { }; stake::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; - stake::on_term_close(block.state_mut(), last_term_finished_block_num, &inactive_validators)?; + stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; Ok(()) } @@ -342,22 +337,16 @@ impl ConsensusEngine for Tendermint { } } -fn block_number_if_term_changed( - header: &Header, - parent_header: &Header, - common_params: &CommonParams, -) -> Option { +fn is_term_changed(header: &Header, parent: &Header, common_params: &CommonParams) -> bool { let term_seconds = common_params.term_seconds(); if term_seconds == 0 { - return None + return false } let current_term_period = header.timestamp() / term_seconds; - let parent_term_period = parent_header.timestamp() / term_seconds; - if current_term_period == parent_term_period { - return None - } - Some(header.number()) + let parent_term_period = parent.timestamp() / term_seconds; + + current_term_period != parent_term_period } fn inactive_validators( From 414007e9d17ed8f92d4eb355d10cae23309ca812 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Mon, 25 Nov 2019 21:13:28 +0900 Subject: [PATCH 03/20] Refactor `is_term_changed` check in on_close_block --- core/src/consensus/tendermint/engine.rs | 149 +++++++++++++----------- 1 file changed, 79 insertions(+), 70 deletions(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 188e554899..fb76d2baf3 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -162,82 +162,92 @@ impl ConsensusEngine for Tendermint { let block_author_reward = total_reward - total_min_fee + distributor.remaining_fee(); let metadata = block.state().metadata()?.expect("Metadata must exist"); - if metadata.current_term_id() == 0 { - self.machine.add_balance(block, &author, block_author_reward)?; + let term = metadata.current_term_id(); + let term_seconds = match term { + 0 => parent_common_params.term_seconds(), + _ => term_common_params.expect("TermCommonParams should exist").term_seconds(), + }; - if is_term_changed(block.header(), parent_header, parent_common_params) { - // First term change - stake::on_term_close(block.state_mut(), block_number, &[])?; + match term { + 0 => { + self.machine.add_balance(block, &author, block_author_reward)?; + } + _ => { + stake::update_validator_weights(&mut block.state_mut(), &author)?; + stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; } - return Ok(()) } - stake::update_validator_weights(&mut block.state_mut(), &author)?; - - stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; - - let term_common_params = term_common_params.expect("TermCommonParams should exist"); - if !is_term_changed(block.header(), parent_header, term_common_params) { + if !is_term_changed(block.header(), parent_header, term_seconds) { return Ok(()) } - let rewards = stake::drain_previous_rewards(&mut 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 inactive_validators = if metadata.current_term_id() == 1 { - assert!(rewards.is_empty()); - - let validators = stake::Validators::load_from_state(block.state())? - .into_iter() - .map(|val| public_to_address(val.pubkey())) - .collect(); - inactive_validators(&*client, start_of_the_current_term, block.header(), validators) - } else { - let start_of_the_previous_term = { - let end_of_the_two_level_previous_term = - client.last_term_finished_block_num((metadata.last_term_finished_block_num() - 1).into()).unwrap(); - - end_of_the_two_level_previous_term + 1 - }; - - let banned = stake::Banned::load_from_state(block.state())?; - let start_of_the_current_term_header = if block_number == start_of_the_current_term { - encoded::Header::new(block.header().clone().rlp_bytes().to_vec()) - } else { - client.block_header(&start_of_the_current_term.into()).unwrap() - }; - - let pending_rewards = calculate_pending_rewards_of_the_previous_term( - &*client, - &*self.validators, - rewards, - start_of_the_current_term, - start_of_the_current_term_header, - start_of_the_previous_term, - &banned, - )?; - - for (address, reward) in pending_rewards { - self.machine.add_balance(block, &address, reward)?; + match term { + 0 => { + // First term change + stake::on_term_close(block.state_mut(), block_number, &[])?; } - - let validators = stake::Validators::load_from_state(block.state())? - .into_iter() - .map(|val| public_to_address(val.pubkey())) - .collect(); - inactive_validators(&*client, start_of_the_current_term, block.header(), validators) - }; - - stake::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; - stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; - + _ => { + let rewards = stake::drain_previous_rewards(&mut 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 inactive_validators = if term == 1 { + assert!(rewards.is_empty()); + + let validators = stake::Validators::load_from_state(block.state())? + .into_iter() + .map(|val| public_to_address(val.pubkey())) + .collect(); + inactive_validators(&*client, start_of_the_current_term, block.header(), validators) + } else { + let start_of_the_previous_term = { + let end_of_the_two_level_previous_term = client + .last_term_finished_block_num((metadata.last_term_finished_block_num() - 1).into()) + .unwrap(); + + end_of_the_two_level_previous_term + 1 + }; + + let banned = stake::Banned::load_from_state(block.state())?; + let start_of_the_current_term_header = if block_number == start_of_the_current_term { + encoded::Header::new(block.header().clone().rlp_bytes().to_vec()) + } else { + client.block_header(&start_of_the_current_term.into()).unwrap() + }; + + let pending_rewards = calculate_pending_rewards_of_the_previous_term( + &*client, + &*self.validators, + rewards, + start_of_the_current_term, + start_of_the_current_term_header, + start_of_the_previous_term, + &banned, + )?; + + for (address, reward) in pending_rewards { + self.machine.add_balance(block, &address, reward)?; + } + + let validators = stake::Validators::load_from_state(block.state())? + .into_iter() + .map(|val| public_to_address(val.pubkey())) + .collect(); + inactive_validators(&*client, start_of_the_current_term, block.header(), validators) + }; + + stake::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; + stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; + } + } Ok(()) } @@ -337,8 +347,7 @@ impl ConsensusEngine for Tendermint { } } -fn is_term_changed(header: &Header, parent: &Header, common_params: &CommonParams) -> bool { - let term_seconds = common_params.term_seconds(); +fn is_term_changed(header: &Header, parent: &Header, term_seconds: u64) -> bool { if term_seconds == 0 { return false } From c36e741a627d84ed88362adde4691cfa0c43ae4c Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Mon, 25 Nov 2019 21:18:36 +0900 Subject: [PATCH 04/20] Refactor `on_term_close` call in on_close_block --- core/src/consensus/tendermint/engine.rs | 38 +++++++++---------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index fb76d2baf3..1f78c80b23 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -182,14 +182,10 @@ impl ConsensusEngine for Tendermint { return Ok(()) } - match term { - 0 => { - // First term change - stake::on_term_close(block.state_mut(), block_number, &[])?; - } + let inactive_validators = match term { + 0 => Vec::new(), _ => { let rewards = stake::drain_previous_rewards(&mut block.state_mut())?; - let start_of_the_current_term = metadata.last_term_finished_block_num() + 1; let client = self .client @@ -199,15 +195,7 @@ impl ConsensusEngine for Tendermint { .upgrade() .ok_or(EngineError::CannotOpenBlock)?; - let inactive_validators = if term == 1 { - assert!(rewards.is_empty()); - - let validators = stake::Validators::load_from_state(block.state())? - .into_iter() - .map(|val| public_to_address(val.pubkey())) - .collect(); - inactive_validators(&*client, start_of_the_current_term, block.header(), validators) - } else { + if term > 1 { let start_of_the_previous_term = { let end_of_the_two_level_previous_term = client .last_term_finished_block_num((metadata.last_term_finished_block_num() - 1).into()) @@ -236,18 +224,20 @@ impl ConsensusEngine for Tendermint { for (address, reward) in pending_rewards { self.machine.add_balance(block, &address, reward)?; } - - let validators = stake::Validators::load_from_state(block.state())? - .into_iter() - .map(|val| public_to_address(val.pubkey())) - .collect(); - inactive_validators(&*client, start_of_the_current_term, block.header(), validators) - }; + } stake::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; - stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; + + let validators = stake::Validators::load_from_state(block.state())? + .into_iter() + .map(|val| public_to_address(val.pubkey())) + .collect(); + inactive_validators(&*client, start_of_the_current_term, block.header(), validators) } - } + }; + + stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; + Ok(()) } From dc4517bac3d094f20f17909576e7d10ffd0a48f5 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Mon, 25 Nov 2019 21:24:27 +0900 Subject: [PATCH 05/20] Remove unnecessary mutable borrow --- core/src/consensus/tendermint/engine.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 1f78c80b23..4cb410751a 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -173,7 +173,7 @@ impl ConsensusEngine for Tendermint { self.machine.add_balance(block, &author, block_author_reward)?; } _ => { - stake::update_validator_weights(&mut block.state_mut(), &author)?; + stake::update_validator_weights(block.state_mut(), &author)?; stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; } } @@ -185,7 +185,7 @@ impl ConsensusEngine for Tendermint { let inactive_validators = match term { 0 => Vec::new(), _ => { - let rewards = stake::drain_previous_rewards(&mut block.state_mut())?; + 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 @@ -226,7 +226,7 @@ impl ConsensusEngine for Tendermint { } } - stake::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; + stake::move_current_to_previous_intermediate_rewards(block.state_mut())?; let validators = stake::Validators::load_from_state(block.state())? .into_iter() From 902265aad56fe52b62425771886335254fe6b69f Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Mon, 25 Nov 2019 17:42:27 +0900 Subject: [PATCH 06/20] Add `era` to CommonParams --- json/src/scheme/params.rs | 79 ++++++++++++++++++++++ types/src/common_params.rs | 133 ++++++++++++++++++++++++++++++++++--- 2 files changed, 201 insertions(+), 11 deletions(-) diff --git a/json/src/scheme/params.rs b/json/src/scheme/params.rs index 2244961556..500b2eb26a 100644 --- a/json/src/scheme/params.rs +++ b/json/src/scheme/params.rs @@ -66,6 +66,10 @@ pub struct Params { pub delegation_threshold: Option, pub min_deposit: Option, pub max_candidate_metadata_size: Option, + + /// A monotonically increasing number to denote the consensus version. + /// It is increased when we fork. + pub era: Option, } #[cfg(test)] @@ -277,4 +281,79 @@ mod tests { assert_eq!(deserialized.min_deposit, Some(32.into())); assert_eq!(deserialized.max_candidate_metadata_size, Some(33.into())); } + + #[test] + #[allow(clippy::cognitive_complexity)] + fn params_deserialization_with_era() { + let s = r#"{ + "maxExtraDataSize": "0x20", + "maxAssetSchemeMetadataSize": "0x0400", + "maxTransferMetadataSize": "0x0100", + "maxTextContentSize": "0x0200", + "networkID" : "tc", + "minPayCost" : 10, + "minSetRegularKeyCost" : 11, + "minCreateShardCost" : 12, + "minSetShardOwnersCost" : 13, + "minSetShardUsersCost" : 14, + "minWrapCccCost" : 15, + "minCustomCost" : 16, + "minStoreCost" : 17, + "minRemoveCost" : 18, + "minMintAssetCost" : 19, + "minTransferAssetCost" : 20, + "minChangeAssetSchemeCost" : 21, + "minComposeAssetCost" : 22, + "minDecomposeAssetCost" : 23, + "minUnwrapCccCost" : 24, + "minIncreaseAssetSupplyCost": 25, + "maxBodySize" : 4194304, + "snapshotPeriod": 16384, + "termSeconds": 3600, + "nominationExpiration": 26, + "custodyPeriod": 27, + "releasePeriod": 28, + "maxNumOfValidators": 29, + "minNumOfValidators": 30, + "delegationThreshold": 31, + "minDeposit": 32, + "maxCandidateMetadataSize": 33, + "era": 34 + }"#; + + let deserialized: Params = serde_json::from_str(s).unwrap(); + assert_eq!(deserialized.max_extra_data_size, 0x20.into()); + assert_eq!(deserialized.max_asset_scheme_metadata_size, 0x0400.into()); + assert_eq!(deserialized.max_transfer_metadata_size, 0x0100.into()); + assert_eq!(deserialized.max_text_content_size, 0x0200.into()); + assert_eq!(deserialized.network_id, "tc".into()); + assert_eq!(deserialized.min_pay_cost, 10.into()); + assert_eq!(deserialized.min_set_regular_key_cost, 11.into()); + assert_eq!(deserialized.min_create_shard_cost, 12.into()); + assert_eq!(deserialized.min_set_shard_owners_cost, 13.into()); + assert_eq!(deserialized.min_set_shard_users_cost, 14.into()); + assert_eq!(deserialized.min_wrap_ccc_cost, 15.into()); + assert_eq!(deserialized.min_custom_cost, 16.into()); + assert_eq!(deserialized.min_store_cost, 17.into()); + assert_eq!(deserialized.min_remove_cost, 18.into()); + assert_eq!(deserialized.min_mint_asset_cost, 19.into()); + assert_eq!(deserialized.min_transfer_asset_cost, 20.into()); + assert_eq!(deserialized.min_change_asset_scheme_cost, 21.into()); + assert_eq!(deserialized.min_compose_asset_cost, 22.into()); + assert_eq!(deserialized.min_decompose_asset_cost, 23.into()); + assert_eq!(deserialized.min_unwrap_ccc_cost, 24.into()); + assert_eq!(deserialized.min_increase_asset_supply_cost, 25.into()); + assert_eq!(deserialized.max_body_size, 4_194_304.into()); + assert_eq!(deserialized.snapshot_period, 16_384.into()); + assert_eq!(deserialized.term_seconds, Some(3600.into())); + assert_eq!(deserialized.nomination_expiration, Some(26.into())); + assert_eq!(deserialized.custody_period, Some(27.into())); + assert_eq!(deserialized.release_period, Some(28.into())); + assert_eq!(deserialized.max_num_of_validators, Some(29.into())); + assert_eq!(deserialized.min_num_of_validators, Some(30.into())); + assert_eq!(deserialized.delegation_threshold, Some(31.into())); + assert_eq!(deserialized.min_deposit, Some(32.into())); + assert_eq!(deserialized.max_candidate_metadata_size, Some(33.into())); + assert_eq!(deserialized.era, Some(34.into())); + } } diff --git a/types/src/common_params.rs b/types/src/common_params.rs index f81e690f7d..7dfa744e25 100644 --- a/types/src/common_params.rs +++ b/types/src/common_params.rs @@ -64,6 +64,8 @@ pub struct CommonParams { delegation_threshold: u64, min_deposit: u64, max_candidate_metadata_size: usize, + + era: u64, } impl CommonParams { @@ -170,6 +172,10 @@ impl CommonParams { self.max_candidate_metadata_size } + pub fn era(&self) -> u64 { + self.era + } + pub fn verify(&self) -> Result<(), String> { if self.term_seconds != 0 { if self.nomination_expiration == 0 { @@ -218,13 +224,21 @@ impl CommonParams { const DEFAULT_PARAMS_SIZE: usize = 23; const NUMBER_OF_STAKE_PARAMS: usize = 9; +const NUMBER_OF_ERA_PARAMS: usize = 1; +const STAKE_PARAM_SIZE: usize = DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS; +const ERA_PARAM_SIZE: usize = STAKE_PARAM_SIZE + NUMBER_OF_ERA_PARAMS; + +const VALID_SIZE: &[usize] = &[DEFAULT_PARAMS_SIZE, STAKE_PARAM_SIZE, ERA_PARAM_SIZE]; impl From for CommonParams { fn from(p: Params) -> Self { - let mut size = DEFAULT_PARAMS_SIZE; - if p.term_seconds.is_some() { - size += NUMBER_OF_STAKE_PARAMS; - } + let size = if p.era.is_some() { + ERA_PARAM_SIZE + } else if p.term_seconds.is_some() { + STAKE_PARAM_SIZE + } else { + DEFAULT_PARAMS_SIZE + }; Self { size, max_extra_data_size: p.max_extra_data_size.into(), @@ -259,6 +273,7 @@ impl From for CommonParams { delegation_threshold: p.delegation_threshold.map(From::from).unwrap_or_default(), min_deposit: p.min_deposit.map(From::from).unwrap_or_default(), max_candidate_metadata_size: p.max_candidate_metadata_size.map(From::from).unwrap_or_default(), + era: p.era.map(From::from).unwrap_or_default(), } } } @@ -292,7 +307,7 @@ impl From for Params { snapshot_period: p.snapshot_period().into(), ..Default::default() }; - if p.size == DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS { + if p.size >= STAKE_PARAM_SIZE { result.term_seconds = Some(p.term_seconds().into()); result.nomination_expiration = Some(p.nomination_expiration().into()); result.custody_period = Some(p.custody_period().into()); @@ -303,13 +318,15 @@ impl From for Params { result.min_deposit = Some(p.min_deposit().into()); result.max_candidate_metadata_size = Some(p.max_candidate_metadata_size().into()); } + if p.size >= ERA_PARAM_SIZE { + result.era = Some(p.era().into()); + } result } } impl Encodable for CommonParams { fn rlp_append(&self, s: &mut RlpStream) { - const VALID_SIZE: &[usize] = &[DEFAULT_PARAMS_SIZE, DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS]; assert!(VALID_SIZE.contains(&self.size), "{} must be in {:?}", self.size, VALID_SIZE); s.begin_list(self.size) .append(&self.max_extra_data_size) @@ -335,7 +352,7 @@ impl Encodable for CommonParams { .append(&self.min_asset_unwrap_ccc_cost) .append(&self.max_body_size) .append(&self.snapshot_period); - if self.size == DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS { + if self.size >= STAKE_PARAM_SIZE { s.append(&self.term_seconds) .append(&self.nomination_expiration) .append(&self.custody_period) @@ -346,13 +363,15 @@ impl Encodable for CommonParams { .append(&self.min_deposit) .append(&self.max_candidate_metadata_size); } + if self.size >= ERA_PARAM_SIZE { + s.append(&self.era); + } } } impl Decodable for CommonParams { fn decode(rlp: &Rlp) -> Result { let size = rlp.item_count()?; - const VALID_SIZE: &[usize] = &[DEFAULT_PARAMS_SIZE, DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS]; if !VALID_SIZE.contains(&size) { return Err(DecoderError::RlpIncorrectListLen { expected: DEFAULT_PARAMS_SIZE, @@ -394,7 +413,7 @@ impl Decodable for CommonParams { delegation_threshold, min_deposit, max_candidate_metadata_size, - ) = if size >= DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS { + ) = if size >= STAKE_PARAM_SIZE { ( rlp.val_at(23)?, rlp.val_at(24)?, @@ -409,6 +428,13 @@ impl Decodable for CommonParams { } else { Default::default() }; + + let era = if size >= ERA_PARAM_SIZE { + rlp.val_at(32)? + } else { + Default::default() + }; + Ok(Self { size, max_extra_data_size, @@ -443,6 +469,7 @@ impl Decodable for CommonParams { delegation_threshold, min_deposit, max_candidate_metadata_size, + era, }) } } @@ -514,7 +541,7 @@ mod tests { #[test] fn rlp_with_extra_fields() { let mut params = CommonParams::default_for_test(); - params.size = DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS; + params.size = ERA_PARAM_SIZE; params.term_seconds = 100; params.min_deposit = 123; rlp_encode_and_decode_test!(params); @@ -524,7 +551,7 @@ mod tests { fn rlp_encoding_are_different_if_the_size_are_different() { let origin = CommonParams::default_for_test(); let mut params = origin; - params.size = DEFAULT_PARAMS_SIZE + NUMBER_OF_STAKE_PARAMS; + params.size = ERA_PARAM_SIZE; assert_ne!(rlp::encode(&origin), rlp::encode(¶ms)); } @@ -591,6 +618,7 @@ mod tests { assert_eq!(deserialized.delegation_threshold, 0); assert_eq!(deserialized.min_deposit, 0); assert_eq!(deserialized.max_candidate_metadata_size, 0); + assert_eq!(deserialized.era, 0); assert_eq!(params, deserialized.into()); } @@ -627,6 +655,7 @@ mod tests { let params = serde_json::from_str::(s).unwrap(); let deserialized = CommonParams::from(params.clone()); + assert_eq!(deserialized.size, STAKE_PARAM_SIZE); assert_eq!(deserialized.max_extra_data_size, 0x20); assert_eq!(deserialized.max_asset_scheme_metadata_size, 0x0400); assert_eq!(deserialized.max_transfer_metadata_size, 0x0100); @@ -659,6 +688,7 @@ mod tests { assert_eq!(deserialized.delegation_threshold, 0); assert_eq!(deserialized.min_deposit, 0); assert_eq!(deserialized.max_candidate_metadata_size, 0); + assert_eq!(deserialized.era, 0); assert_eq!( Params { @@ -670,6 +700,7 @@ mod tests { delegation_threshold: Some(0.into()), min_deposit: Some(0.into()), max_candidate_metadata_size: Some(0.into()), + era: None, ..params }, deserialized.into(), @@ -716,6 +747,85 @@ mod tests { }"#; let params = serde_json::from_str::(s).unwrap(); let deserialized = CommonParams::from(params.clone()); + assert_eq!(deserialized.size, STAKE_PARAM_SIZE); + assert_eq!(deserialized.max_extra_data_size, 0x20); + assert_eq!(deserialized.max_asset_scheme_metadata_size, 0x0400); + assert_eq!(deserialized.max_transfer_metadata_size, 0x0100); + assert_eq!(deserialized.max_text_content_size, 0x0200); + assert_eq!(deserialized.network_id, "tc".into()); + assert_eq!(deserialized.min_pay_transaction_cost, 10); + assert_eq!(deserialized.min_set_regular_key_transaction_cost, 11); + assert_eq!(deserialized.min_create_shard_transaction_cost, 12); + assert_eq!(deserialized.min_set_shard_owners_transaction_cost, 13); + assert_eq!(deserialized.min_set_shard_users_transaction_cost, 14); + assert_eq!(deserialized.min_wrap_ccc_transaction_cost, 15); + assert_eq!(deserialized.min_custom_transaction_cost, 16); + assert_eq!(deserialized.min_store_transaction_cost, 17); + assert_eq!(deserialized.min_remove_transaction_cost, 18); + assert_eq!(deserialized.min_asset_mint_cost, 19); + assert_eq!(deserialized.min_asset_transfer_cost, 20); + assert_eq!(deserialized.min_asset_scheme_change_cost, 21); + assert_eq!(deserialized.min_asset_compose_cost, 22); + assert_eq!(deserialized.min_asset_decompose_cost, 23); + assert_eq!(deserialized.min_asset_unwrap_ccc_cost, 24); + assert_eq!(deserialized.min_asset_supply_increase_cost, 25); + assert_eq!(deserialized.max_body_size, 4_194_304); + assert_eq!(deserialized.snapshot_period, 16_384); + assert_eq!(deserialized.term_seconds, 3600); + assert_eq!(deserialized.nomination_expiration, 26); + assert_eq!(deserialized.custody_period, 27); + assert_eq!(deserialized.release_period, 28); + assert_eq!(deserialized.max_num_of_validators, 29); + assert_eq!(deserialized.min_num_of_validators, 30); + assert_eq!(deserialized.delegation_threshold, 31); + assert_eq!(deserialized.min_deposit, 32); + assert_eq!(deserialized.max_candidate_metadata_size, 33); + assert_eq!(deserialized.era, 0); + + assert_eq!(params, deserialized.into()); + } + + #[test] + #[allow(clippy::cognitive_complexity)] + fn params_from_json_with_era() { + let s = r#"{ + "maxExtraDataSize": "0x20", + "maxAssetSchemeMetadataSize": "0x0400", + "maxTransferMetadataSize": "0x0100", + "maxTextContentSize": "0x0200", + "networkID" : "tc", + "minPayCost" : 10, + "minSetRegularKeyCost" : 11, + "minCreateShardCost" : 12, + "minSetShardOwnersCost" : 13, + "minSetShardUsersCost" : 14, + "minWrapCccCost" : 15, + "minCustomCost" : 16, + "minStoreCost" : 17, + "minRemoveCost" : 18, + "minMintAssetCost" : 19, + "minTransferAssetCost" : 20, + "minChangeAssetSchemeCost" : 21, + "minComposeAssetCost" : 22, + "minDecomposeAssetCost" : 23, + "minUnwrapCccCost" : 24, + "minIncreaseAssetSupplyCost": 25, + "maxBodySize" : 4194304, + "snapshotPeriod": 16384, + "termSeconds": 3600, + "nominationExpiration": 26, + "custodyPeriod": 27, + "releasePeriod": 28, + "maxNumOfValidators": 29, + "minNumOfValidators": 30, + "delegationThreshold": 31, + "minDeposit": 32, + "maxCandidateMetadataSize": 33, + "era": 34 + }"#; + let params = serde_json::from_str::(s).unwrap(); + let deserialized = CommonParams::from(params.clone()); + assert_eq!(deserialized.size, ERA_PARAM_SIZE); assert_eq!(deserialized.max_extra_data_size, 0x20); assert_eq!(deserialized.max_asset_scheme_metadata_size, 0x0400); assert_eq!(deserialized.max_transfer_metadata_size, 0x0100); @@ -748,6 +858,7 @@ mod tests { assert_eq!(deserialized.delegation_threshold, 31); assert_eq!(deserialized.min_deposit, 32); assert_eq!(deserialized.max_candidate_metadata_size, 33); + assert_eq!(deserialized.era, 34); assert_eq!(params, deserialized.into()); } From 114b058c2a1843fcddc4b367ffba74c9c68b1038 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Tue, 26 Nov 2019 11:35:41 +0900 Subject: [PATCH 07/20] Extract a function to verify a new CommonParams compare to the current params --- core/src/consensus/stake/actions.rs | 10 +--------- types/src/common_params.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/core/src/consensus/stake/actions.rs b/core/src/consensus/stake/actions.rs index a0f63c4dbf..912461813b 100644 --- a/core/src/consensus/stake/actions.rs +++ b/core/src/consensus/stake/actions.rs @@ -105,15 +105,7 @@ impl Action { params, signatures, } => { - let current_network_id = current_params.network_id(); - let transaction_network_id = params.network_id(); - if current_network_id != transaction_network_id { - return Err(SyntaxError::InvalidCustomAction(format!( - "The current network id is {} but the transaction tries to change the network id to {}", - current_network_id, transaction_network_id - ))) - } - params.verify().map_err(SyntaxError::InvalidCustomAction)?; + params.verify_change(current_params).map_err(SyntaxError::InvalidCustomAction)?; let action = Action::ChangeParams { metadata_seq: *metadata_seq, params: params.clone(), diff --git a/types/src/common_params.rs b/types/src/common_params.rs index 7dfa744e25..6e8d38f67c 100644 --- a/types/src/common_params.rs +++ b/types/src/common_params.rs @@ -220,6 +220,19 @@ impl CommonParams { } Ok(()) } + + pub fn verify_change(&self, current_params: &Self) -> Result<(), String> { + self.verify()?; + let current_network_id = current_params.network_id(); + let transaction_network_id = self.network_id(); + if current_network_id != transaction_network_id { + return Err(format!( + "The current network id is {} but the transaction tries to change the network id to {}", + current_network_id, transaction_network_id + )) + } + Ok(()) + } } const DEFAULT_PARAMS_SIZE: usize = 23; From f83de3d64d01efa16bed8c5ee7184ec4a6a8efe4 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Tue, 26 Nov 2019 11:37:47 +0900 Subject: [PATCH 08/20] Make era nondecreasing --- types/src/common_params.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/types/src/common_params.rs b/types/src/common_params.rs index 6e8d38f67c..e59c1eab9a 100644 --- a/types/src/common_params.rs +++ b/types/src/common_params.rs @@ -231,6 +231,9 @@ impl CommonParams { current_network_id, transaction_network_id )) } + if self.era < current_params.era { + return Err(format!("The era({}) shouldn't be less than the current era({})", self.era, current_params.era)) + } Ok(()) } } From d1e1c7e18ee13ee0645f4dde7099f355de0e051e Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 17:43:42 +0900 Subject: [PATCH 09/20] 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 f59616910f25d2f91506aff84711e3478dfe480c Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 18:17:08 +0900 Subject: [PATCH 10/20] 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 bb0d9684a074b92da39017ef344528b1dc8ca433 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 18:35:17 +0900 Subject: [PATCH 11/20] 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 { From a85b60a19c8ec25ca439366c88c4c476c49a40a5 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Tue, 19 Nov 2019 10:23:43 +0900 Subject: [PATCH 12/20] Refactor term_common_params method --- core/src/block.rs | 12 ++---------- core/src/client/client.rs | 9 +++++++++ core/src/client/mod.rs | 1 + core/src/client/test_client.rs | 4 ++++ core/src/miner/miner.rs | 11 +---------- 5 files changed, 17 insertions(+), 20 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index 7d5a5dea68..9a6584d090 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -33,6 +33,7 @@ use crate::client::{EngineInfo, TermInfo}; use crate::consensus::CodeChainEngine; use crate::error::{BlockError, Error}; use crate::transaction::{SignedTransaction, UnverifiedTransaction}; +use crate::BlockId; /// A block, encoded as it is on the block chain. #[derive(Debug, Clone, PartialEq)] @@ -493,16 +494,7 @@ pub fn enact( b.populate_from(header); b.push_transactions(transactions, client, parent.number(), parent.timestamp())?; - let term_common_params = { - let block_number = client - .last_term_finished_block_num((*header.parent_hash()).into()) - .expect("The block of the parent hash should exist"); - if block_number == 0 { - None - } else { - Some(client.common_params((block_number).into()).expect("Common params should exist")) - } - }; + let term_common_params = client.term_common_params(BlockId::Hash(*header.parent_hash())); b.close_and_lock(parent, term_common_params.as_ref()) } diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 2389a88235..32cdb03506 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -826,6 +826,15 @@ impl TermInfo for Client { .map(|state| state.metadata().unwrap().expect("Metadata always exist")) .map(|metadata| metadata.current_term_id()) } + + fn term_common_params(&self, id: BlockId) -> Option { + let block_number = self.last_term_finished_block_num(id).expect("The block of the parent hash should exist"); + if block_number == 0 { + None + } else { + Some(self.common_params((block_number).into()).expect("Common params should exist")) + } + } } impl AccountData for Client { diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index eb3adaf3b3..edb8abc76f 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -119,6 +119,7 @@ pub trait ConsensusClient: BlockChainClient + EngineClient + EngineInfo + TermIn pub trait TermInfo { fn last_term_finished_block_num(&self, id: BlockId) -> Option; fn current_term_id(&self, id: BlockId) -> Option; + fn term_common_params(&self, id: BlockId) -> Option; } /// Provides methods to access account info diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 31775914f8..f14c2bf80a 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -679,6 +679,10 @@ impl TermInfo for TestBlockChainClient { fn current_term_id(&self, _id: BlockId) -> Option { self.term_id } + + fn term_common_params(&self, _id: BlockId) -> Option { + None + } } impl StateInfo for TestBlockChainClient { diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 6dc64bb987..08a2ba0a99 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -603,16 +603,7 @@ impl Miner { let parent_header = chain.block_header(&parent_hash.into()).expect("Parent header MUST exist"); (parent_header.decode(), parent_hash) }; - let term_common_params = { - let block_number = chain - .last_term_finished_block_num(parent_hash.into()) - .expect("The block of the parent hash should exist"); - if block_number == 0 { - None - } else { - Some(chain.common_params((block_number).into()).expect("Common params should exist")) - } - }; + let term_common_params = chain.term_common_params(parent_hash.into()); let block = open_block.close(&parent_header, term_common_params.as_ref())?; let fetch_seq = |p: &Public| { From 4634de4beedb8f9b198b1bbc57e566b1cf37e490 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Tue, 26 Nov 2019 11:21:29 +0900 Subject: [PATCH 13/20] Add term_params to Metadata `term_params` should be snapshot every `on_term_close` --- state/src/item/metadata.rs | 136 +++++++++++++++++++++++++------------ 1 file changed, 94 insertions(+), 42 deletions(-) diff --git a/state/src/item/metadata.rs b/state/src/item/metadata.rs index 6ad74076b3..39651888a4 100644 --- a/state/src/item/metadata.rs +++ b/state/src/item/metadata.rs @@ -34,6 +34,7 @@ pub struct Metadata { term: TermMetadata, seq: u64, params: Option, + term_params: Option, } impl Metadata { @@ -45,6 +46,7 @@ impl Metadata { term: Default::default(), seq: 0, params: None, + term_params: None, } } @@ -93,6 +95,14 @@ impl Metadata { self.params = Some(params); } + pub fn term_params(&self) -> Option<&CommonParams> { + self.term_params.as_ref() + } + + pub fn snapshot_term_params(&mut self) { + self.term_params = self.params; + } + pub fn increase_term_id(&mut self, last_term_finished_block_num: u64) { assert!(self.term.last_term_finished_block_num < last_term_finished_block_num); self.term.last_term_finished_block_num = last_term_finished_block_num; @@ -124,25 +134,31 @@ impl CacheableItem for Metadata { const PREFIX: u8 = super::METADATA_PREFIX; +const INITIAL_LEN: usize = 4; +const TERM_LEN: usize = INITIAL_LEN + 2; +const PARAMS_LEN: usize = TERM_LEN + 2; +const TERM_PARAMS_LEN: usize = PARAMS_LEN + 1; +const VALID_LEN: &[usize] = &[INITIAL_LEN, TERM_LEN, PARAMS_LEN, TERM_PARAMS_LEN]; + 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 { - if !term_changed { - len += TERM_LEN; + let term_params_exist = self.term_params.is_some(); + + let len = if term_params_exist { + if !params_changed { + panic!("Term params only can be changed if params changed"); } - len += PARAMS_LEN; - } + TERM_PARAMS_LEN + } else if params_changed { + PARAMS_LEN + } else if term_changed { + TERM_LEN + } else { + INITIAL_LEN + }; + s.begin_list(len) .append(&PREFIX) .append(&self.number_of_shards) @@ -159,48 +175,63 @@ impl Encodable for Metadata { } s.append(&self.seq).append(self.params.as_ref().unwrap()); } + if term_params_exist { + if !params_changed { + unreachable!("Term params only can be changed if params changed"); + } + s.append(self.term_params.as_ref().unwrap()); + } } } impl Decodable for Metadata { fn decode(rlp: &Rlp) -> Result { - 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, - expected: 4, - }) - } - }; + let item_count = rlp.item_count()?; + if !VALID_LEN.contains(&item_count) { + return Err(DecoderError::RlpInvalidLength { + got: item_count, + expected: 4, + }) + } + let prefix = rlp.val_at::(0)?; if PREFIX != prefix { cdebug!(STATE, "{} is not an expected prefix for asset", prefix); return Err(DecoderError::Custom("Unexpected prefix")) } + let number_of_shards = rlp.val_at(1)?; + let number_of_initial_shards = rlp.val_at(2)?; + let hashes = rlp.list_at(3)?; + + let term = if item_count >= TERM_LEN { + TermMetadata { + last_term_finished_block_num: rlp.val_at(4)?, + current_term_id: rlp.val_at(5)?, + } + } else { + TermMetadata::default() + }; + + let (seq, params) = if item_count >= PARAMS_LEN { + (rlp.val_at(6)?, Some(rlp.val_at(7)?)) + } else { + Default::default() + }; + + let term_params = if item_count >= TERM_PARAMS_LEN { + Some(rlp.val_at(8)?) + } else { + Default::default() + }; + Ok(Self { - number_of_shards: rlp.val_at(1)?, - number_of_initial_shards: rlp.val_at(2)?, - hashes: rlp.list_at(3)?, + number_of_shards, + number_of_initial_shards, + hashes, term, seq, params, + term_params, }) } } @@ -266,6 +297,7 @@ mod tests { term: Default::default(), seq: 0, params: None, + term_params: None, }; let mut rlp = RlpStream::new_list(4); rlp.append(&PREFIX).append(&10u16).append(&1u16).append_list::(&[]); @@ -281,6 +313,7 @@ mod tests { term: Default::default(), seq: 3, params: Some(CommonParams::default_for_test()), + term_params: Some(CommonParams::default_for_test()), }; rlp_encode_and_decode_test!(metadata); } @@ -297,6 +330,7 @@ mod tests { }, seq: 0, params: None, + term_params: None, }; rlp_encode_and_decode_test!(metadata); } @@ -313,6 +347,24 @@ mod tests { }, seq: 3, params: Some(CommonParams::default_for_test()), + term_params: Some(CommonParams::default_for_test()), + }; + rlp_encode_and_decode_test!(metadata); + } + + #[test] + fn metadata_with_term_and_seq_but_not_term_params() { + 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()), + term_params: None, }; rlp_encode_and_decode_test!(metadata); } From 09af46ae2040c9a5e8b42d1ed876bbc1ef8a2bbd Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Tue, 26 Nov 2019 14:43:37 +0900 Subject: [PATCH 14/20] Snapshot term_metadata on every term close for the era > 0 --- core/src/consensus/tendermint/engine.rs | 10 +++++++++- state/src/impls/top_level.rs | 6 ++++++ state/src/traits.rs | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 3ea3282c7a..f5263796c1 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -24,7 +24,7 @@ use std::sync::{Arc, Weak}; use ckey::{public_to_address, Address}; use cnetwork::NetworkService; use crossbeam_channel as crossbeam; -use cstate::{ActionHandler, TopStateView}; +use cstate::{ActionHandler, TopState, TopStateView}; use ctypes::{BlockHash, CommonParams, Header}; use num_rational::Ratio; @@ -235,6 +235,14 @@ impl ConsensusEngine for Tendermint { stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; + match term { + 0 => {} + _ => match term_common_params.expect("Term common params should exist").era() { + 0 => {} + 1 => block.state_mut().snapshot_term_params()?, + _ => unimplemented!("It is not decided how we handle this"), + }, + } Ok(()) } diff --git a/state/src/impls/top_level.rs b/state/src/impls/top_level.rs index b871efaa58..4d51ec0b3f 100644 --- a/state/src/impls/top_level.rs +++ b/state/src/impls/top_level.rs @@ -999,6 +999,12 @@ impl TopState for TopLevelState { metadata.increase_seq(); Ok(()) } + + fn snapshot_term_params(&mut self) -> StateResult<()> { + let mut metadata = self.get_metadata_mut()?; + metadata.snapshot_term_params(); + Ok(()) + } } fn is_active_account(state: &dyn TopStateView, address: &Address) -> TrieResult { diff --git a/state/src/traits.rs b/state/src/traits.rs index 40d94688f7..c9414611d1 100644 --- a/state/src/traits.rs +++ b/state/src/traits.rs @@ -183,6 +183,7 @@ pub trait TopState { fn remove_action_data(&mut self, key: &H256); fn update_params(&mut self, metadata_seq: u64, params: CommonParams) -> StateResult<()>; + fn snapshot_term_params(&mut self) -> StateResult<()>; } pub trait StateWithCache { From 237b7f45b6ddb28682a26d785d5621a7068d9089 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Tue, 26 Nov 2019 17:10:12 +0900 Subject: [PATCH 15/20] Read term_params from the snapshot --- core/src/client/client.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 32cdb03506..eb1d15ff05 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -828,11 +828,19 @@ impl TermInfo for Client { } fn term_common_params(&self, id: BlockId) -> Option { - let block_number = self.last_term_finished_block_num(id).expect("The block of the parent hash should exist"); - if block_number == 0 { - None + let state = self.state_at(id)?; + let metadata = state.metadata().unwrap().expect("Metadata always exist"); + + if let Some(term_params) = metadata.term_params() { + Some(*term_params) } else { - Some(self.common_params((block_number).into()).expect("Common params should exist")) + let block_number = + self.last_term_finished_block_num(id).expect("The block of the parent hash should exist"); + if block_number == 0 { + None + } else { + Some(self.common_params((block_number).into()).expect("Common params should exist")) + } } } } From 6519162f816e38a4360f7b20cb48f343f1a250b3 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Thu, 28 Nov 2019 16:09:31 +0900 Subject: [PATCH 16/20] Add era support to e2e dynval test --- test/src/e2e.dynval/setup.ts | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/test/src/e2e.dynval/setup.ts b/test/src/e2e.dynval/setup.ts index d9c577e824..f8cb3d227a 100644 --- a/test/src/e2e.dynval/setup.ts +++ b/test/src/e2e.dynval/setup.ts @@ -28,7 +28,7 @@ export function withNodes( options: { promiseExpect: PromiseExpect; validators: ValidatorConfig[]; - overrideParams?: Partial; + overrideParams?: Partial; onBeforeEnable?: (nodes: CodeChain[]) => Promise; } ) { @@ -82,7 +82,7 @@ export function findNode(nodes: CodeChain[], signer: Signer) { async function createNodes(options: { promiseExpect: PromiseExpect; validators: ValidatorConfig[]; - initialParams: typeof defaultParams; + initialParams: CommonParams; onBeforeEnable?: (nodes: CodeChain[]) => Promise; }): Promise { const chain = `${__dirname}/../scheme/tendermint-dynval.json`; @@ -347,12 +347,14 @@ export const defaultParams = { maxCandidateMetadataSize: 128 }; -export async function changeParams( - node: CodeChain, - metadataSeq: number, - params: typeof defaultParams -) { - const newParams: any[] = [ +interface EraCommonParams { + era: number; +} + +type CommonParams = typeof defaultParams & Partial; + +function encodeParams(params: CommonParams): any[] { + const result = [ params.maxExtraDataSize, params.maxAssetSchemeMetadataSize, params.maxTransferMetadataSize, @@ -386,12 +388,23 @@ export async function changeParams( params.minDeposit, params.maxCandidateMetadataSize ]; + if (params.era) { + result.push(params.era); + } + return result; +} + +export async function changeParams( + node: CodeChain, + metadataSeq: number, + params: CommonParams +) { const changeParamsActionRlp: [ number, number, (number | string)[], ...string[] - ] = [0xff, metadataSeq, newParams]; + ] = [0xff, metadataSeq, encodeParams(params)]; const message = blake256(RLP.encode(changeParamsActionRlp).toString("hex")); changeParamsActionRlp.push( `0x${SDK.util.signEcdsa(message, faucetSecret)}` From e4377cfb7df3cf405d0a33250aa9defacba66bf3 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Thu, 28 Nov 2019 16:54:48 +0900 Subject: [PATCH 17/20] Add basic era tests --- test/src/e2e.dynval/1/dv.era.test.ts | 86 ++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 test/src/e2e.dynval/1/dv.era.test.ts diff --git a/test/src/e2e.dynval/1/dv.era.test.ts b/test/src/e2e.dynval/1/dv.era.test.ts new file mode 100644 index 0000000000..6323d6db51 --- /dev/null +++ b/test/src/e2e.dynval/1/dv.era.test.ts @@ -0,0 +1,86 @@ +// Copyright 2019 Kodebox, Inc. +// This file is part of CodeChain. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +import * as chai from "chai"; +import { expect } from "chai"; +import * as chaiAsPromised from "chai-as-promised"; +import "mocha"; + +import { validators } from "../../../tendermint.dynval/constants"; +import { PromiseExpect } from "../../helper/promise"; +import { changeParams, setTermTestTimeout, withNodes } from "../setup"; + +chai.use(chaiAsPromised); + +describe("Change era", function() { + const promiseExpect = new PromiseExpect(); + const { nodes, initialParams } = withNodes(this, { + promiseExpect, + overrideParams: { + minNumOfValidators: 3, + maxNumOfValidators: 5 + }, + validators: validators.slice(0, 3).map(signer => ({ + signer, + delegation: 5_000, + deposit: 10_000_000 + })) + }); + + it("should be enabled", async function() { + const termWaiter = setTermTestTimeout(this, { + terms: 3 + }); + + const checkingNode = nodes[0]; + const changeTxHash = await changeParams(checkingNode, 1, { + ...initialParams, + era: 1 + }); + + await checkingNode.waitForTx(changeTxHash); + + await termWaiter.waitNodeUntilTerm(checkingNode, { + target: 3, + termPeriods: 2 + }); + }); + + it("must increase monotonically", async function() { + const termWaiter = setTermTestTimeout(this, { + terms: 2 + }); + + const checkingNode = nodes[0]; + const changeTxHash = await changeParams(checkingNode, 1, { + ...initialParams, + era: 1 + }); + + await checkingNode.waitForTx(changeTxHash); + + await expect( + changeParams(checkingNode, 2, { + ...initialParams, + era: 0 + }) + ).eventually.rejected; + }); + + afterEach(function() { + promiseExpect.checkFulfilled(); + }); +}); From 95c1562f3d21979a318cdd8c9c884f4f2c60233c Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 26 Nov 2019 15:38:21 +0900 Subject: [PATCH 18/20] Introduce on_open_block `on_open_block` is called when the block is created, before processing any transactions included in the block. --- core/src/block.rs | 5 +++++ core/src/consensus/mod.rs | 5 +++++ core/src/consensus/tendermint/engine.rs | 9 +++++++++ core/src/miner/miner.rs | 1 + 4 files changed, 20 insertions(+) diff --git a/core/src/block.rs b/core/src/block.rs index 9a6584d090..ab35efe3cf 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -313,6 +313,10 @@ impl<'x> OpenBlock<'x> { self.block.header.set_seal(seal); Ok(()) } + + pub fn inner_mut(&mut self) -> &mut ExecutedBlock { + &mut self.block + } } /// Just like `OpenBlock`, except that we've applied `Engine::on_close_block`, finished up the non-seal header fields. @@ -492,6 +496,7 @@ pub fn enact( let mut b = OpenBlock::try_new(engine, db, parent, Address::default(), vec![])?; b.populate_from(header); + engine.on_open_block(b.inner_mut())?; b.push_transactions(transactions, client, parent.number(), parent.timestamp())?; let term_common_params = client.term_common_params(BlockId::Hash(*header.parent_hash())); diff --git a/core/src/consensus/mod.rs b/core/src/consensus/mod.rs index c0d78b1485..329e9e2420 100644 --- a/core/src/consensus/mod.rs +++ b/core/src/consensus/mod.rs @@ -221,6 +221,11 @@ pub trait ConsensusEngine: Sync + Send { /// Stops any services that the may hold the Engine and makes it safe to drop. fn stop(&self) {} + /// Block transformation functions, before the transactions. + fn on_open_block(&self, _block: &mut ExecutedBlock) -> Result<(), Error> { + Ok(()) + } + /// Block transformation functions, after the transactions. fn on_close_block( &self, diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index f5263796c1..9bfc17b4a4 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -134,6 +134,15 @@ impl ConsensusEngine for Tendermint { fn stop(&self) {} + /// Block transformation functions, before the transactions. + fn on_open_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + let metadata = block.state().metadata()?.expect("Metadata must exist"); + if block.header().number() == metadata.last_term_finished_block_num() + 1 { + // FIXME: on_term_open + } + Ok(()) + } + fn on_close_block( &self, block: &mut ExecutedBlock, diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 08a2ba0a99..5b7c42d865 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -527,6 +527,7 @@ impl Miner { return Ok(None) } } + self.engine.on_open_block(open_block.inner_mut())?; let mut invalid_transactions = Vec::new(); From be7dbee2d4b37020c5a58704eae911c0da9788ac Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Wed, 27 Nov 2019 18:45:45 +0900 Subject: [PATCH 19/20] Precalculate the fee in era=1 --- core/src/consensus/solo/mod.rs | 6 +- core/src/consensus/stake/action_data.rs | 205 +++++++++++++++++++----- core/src/consensus/stake/mod.rs | 69 ++++++-- core/src/consensus/tendermint/engine.rs | 70 ++++++-- 4 files changed, 278 insertions(+), 72 deletions(-) diff --git a/core/src/consensus/solo/mod.rs b/core/src/consensus/solo/mod.rs index 414e45a00f..77a41dd9bb 100644 --- a/core/src/consensus/solo/mod.rs +++ b/core/src/consensus/solo/mod.rs @@ -118,7 +118,7 @@ impl ConsensusEngine for Solo { self.machine.add_balance(block, &author, block_author_reward)?; return Ok(()) } - stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; + stake::v0::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; let last_term_finished_block_num = { let header = block.header(); let current_term_period = header.timestamp() / term_seconds; @@ -128,8 +128,8 @@ impl ConsensusEngine for Solo { } header.number() }; - stake::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; - let rewards = stake::drain_previous_rewards(&mut block.state_mut())?; + stake::v0::move_current_to_previous_intermediate_rewards(&mut block.state_mut())?; + let rewards = stake::v0::drain_previous_rewards(&mut block.state_mut())?; for (address, reward) in rewards { self.machine.add_balance(block, &address, reward)?; } diff --git a/core/src/consensus/stake/action_data.rs b/core/src/consensus/stake/action_data.rs index acc1add3b3..af14c8b33e 100644 --- a/core/src/consensus/stake/action_data.rs +++ b/core/src/consensus/stake/action_data.rs @@ -18,7 +18,6 @@ use std::cmp::Ordering; use std::collections::btree_map::{BTreeMap, Entry}; use std::collections::btree_set::{self, BTreeSet}; use std::collections::{btree_map, HashMap, HashSet}; -use std::mem; use std::ops::Deref; use std::vec; @@ -408,51 +407,116 @@ impl IntoIterator for Validators { } } -#[derive(Default, Debug, PartialEq)] -pub struct IntermediateRewards { - previous: BTreeMap, - current: BTreeMap, -} +pub mod v0 { + use std::mem; -impl IntermediateRewards { - pub fn load_from_state(state: &TopLevelState) -> StateResult { - let key = get_intermediate_rewards_key(); - let action_data = state.action_data(&key)?; - let (previous, current) = decode_map_tuple(action_data.as_ref()); + use super::*; - Ok(Self { - previous, - current, - }) + #[derive(Default, Debug, PartialEq)] + pub struct IntermediateRewards { + pub(super) previous: BTreeMap, + pub(super) current: BTreeMap, } - pub fn save_to_state(&self, state: &mut TopLevelState) -> StateResult<()> { - let key = get_intermediate_rewards_key(); - if self.previous.is_empty() && self.current.is_empty() { - state.remove_action_data(&key); - } else { - let encoded = encode_map_tuple(&self.previous, &self.current); - state.update_action_data(&key, encoded)?; + impl IntermediateRewards { + pub fn load_from_state(state: &TopLevelState) -> StateResult { + let key = get_intermediate_rewards_key(); + let action_data = state.action_data(&key)?; + let (previous, current) = decode_map_tuple(action_data.as_ref()); + + Ok(Self { + previous, + current, + }) } - Ok(()) - } - pub fn add_quantity(&mut self, address: Address, quantity: StakeQuantity) { - if quantity == 0 { - return + pub fn save_to_state(&self, state: &mut TopLevelState) -> StateResult<()> { + let key = get_intermediate_rewards_key(); + if self.previous.is_empty() && self.current.is_empty() { + state.remove_action_data(&key); + } else { + let encoded = encode_map_tuple(&self.previous, &self.current); + state.update_action_data(&key, encoded)?; + } + Ok(()) + } + + pub fn add_quantity(&mut self, address: Address, quantity: StakeQuantity) { + if quantity == 0 { + return + } + *self.current.entry(address).or_insert(0) += quantity; + } + + pub fn drain_previous(&mut self) -> BTreeMap { + let mut new = BTreeMap::new(); + mem::swap(&mut new, &mut self.previous); + new + } + + pub fn move_current_to_previous(&mut self) { + assert!(self.previous.is_empty()); + mem::swap(&mut self.previous, &mut self.current); } - *self.current.entry(address).or_insert(0) += quantity; } +} + +pub mod v1 { + use std::mem; + + use super::*; - pub fn drain_previous(&mut self) -> BTreeMap { - let mut new = BTreeMap::new(); - mem::swap(&mut new, &mut self.previous); - new + #[derive(Default, Debug, PartialEq)] + pub struct IntermediateRewards { + pub(super) current: BTreeMap, + pub(super) calculated: BTreeMap, } - pub fn move_current_to_previous(&mut self) { - assert!(self.previous.is_empty()); - mem::swap(&mut self.previous, &mut self.current); + impl IntermediateRewards { + pub fn load_from_state(state: &TopLevelState) -> StateResult { + let key = get_intermediate_rewards_key(); + let action_data = state.action_data(&key)?; + let (current, calculated) = decode_map_tuple(action_data.as_ref()); + + Ok(Self { + current, + calculated, + }) + } + + pub fn save_to_state(&self, state: &mut TopLevelState) -> StateResult<()> { + let key = get_intermediate_rewards_key(); + if self.current.is_empty() && self.calculated.is_empty() { + state.remove_action_data(&key); + } else { + let encoded = encode_map_tuple(&self.current, &self.calculated); + state.update_action_data(&key, encoded)?; + } + Ok(()) + } + + pub fn add_quantity(&mut self, address: Address, quantity: StakeQuantity) { + if quantity == 0 { + return + } + *self.current.entry(address).or_insert(0) += quantity; + } + + pub fn update_calculated(&mut self, rewards: BTreeMap) { + self.calculated = rewards; + } + + pub fn drain_current(&mut self) -> BTreeMap { + let mut new = BTreeMap::new(); + mem::swap(&mut new, &mut self.current); + new + } + + pub fn drain_calculated(&mut self) -> BTreeMap { + let mut new = BTreeMap::new(); + mem::swap(&mut new, &mut self.calculated); + new + } } } @@ -1129,39 +1193,39 @@ mod tests { } #[test] - fn load_and_save_intermediate_rewards() { + fn load_and_save_intermediate_rewards_v0() { let mut state = helpers::get_temp_state(); - let rewards = IntermediateRewards::load_from_state(&state).unwrap(); + let rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); rewards.save_to_state(&mut state).unwrap(); } #[test] - fn add_quantity() { + fn add_quantity_v0() { let address1 = Address::random(); let address2 = Address::random(); let mut state = helpers::get_temp_state(); - let mut origin_rewards = IntermediateRewards::load_from_state(&state).unwrap(); + let mut origin_rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); origin_rewards.add_quantity(address1, 1); origin_rewards.add_quantity(address2, 2); origin_rewards.save_to_state(&mut state).unwrap(); - let recovered_rewards = IntermediateRewards::load_from_state(&state).unwrap(); + let recovered_rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); assert_eq!(origin_rewards, recovered_rewards); } #[test] - fn drain() { + fn drain_v0() { let address1 = Address::random(); let address2 = Address::random(); let mut state = helpers::get_temp_state(); - let mut origin_rewards = IntermediateRewards::load_from_state(&state).unwrap(); + let mut origin_rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); origin_rewards.add_quantity(address1, 1); origin_rewards.add_quantity(address2, 2); origin_rewards.save_to_state(&mut state).unwrap(); - let mut recovered_rewards = IntermediateRewards::load_from_state(&state).unwrap(); + let mut recovered_rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); assert_eq!(origin_rewards, recovered_rewards); let _drained = recovered_rewards.drain_previous(); recovered_rewards.save_to_state(&mut state).unwrap(); - let mut final_rewards = IntermediateRewards::load_from_state(&state).unwrap(); + let mut final_rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); assert_eq!(BTreeMap::new(), final_rewards.previous); let current = final_rewards.current.clone(); final_rewards.move_current_to_previous(); @@ -1169,6 +1233,59 @@ mod tests { assert_eq!(current, final_rewards.previous); } + #[test] + fn save_v0_and_load_v1_intermediate_rewards() { + let address1 = Address::random(); + let address2 = Address::random(); + let mut state = helpers::get_temp_state(); + let mut origin_rewards = v0::IntermediateRewards::load_from_state(&state).unwrap(); + origin_rewards.add_quantity(address1, 1); + origin_rewards.add_quantity(address2, 2); + origin_rewards.save_to_state(&mut state).unwrap(); + let recovered_rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + assert_eq!(origin_rewards.previous, recovered_rewards.current); + assert_eq!(origin_rewards.current, recovered_rewards.calculated); + } + + #[test] + fn load_and_save_intermediate_rewards_v1() { + let mut state = helpers::get_temp_state(); + let rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + rewards.save_to_state(&mut state).unwrap(); + } + + #[test] + fn add_quantity_v1() { + let address1 = Address::random(); + let address2 = Address::random(); + let mut state = helpers::get_temp_state(); + let mut origin_rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + origin_rewards.add_quantity(address1, 1); + origin_rewards.add_quantity(address2, 2); + origin_rewards.save_to_state(&mut state).unwrap(); + let recovered_rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + assert_eq!(origin_rewards, recovered_rewards); + } + + #[test] + fn drain_v1() { + let address1 = Address::random(); + let address2 = Address::random(); + let mut state = helpers::get_temp_state(); + let mut origin_rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + origin_rewards.add_quantity(address1, 1); + origin_rewards.add_quantity(address2, 2); + origin_rewards.save_to_state(&mut state).unwrap(); + let mut recovered_rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + assert_eq!(origin_rewards, recovered_rewards); + recovered_rewards.drain_current(); + recovered_rewards.save_to_state(&mut state).unwrap(); + let mut final_rewards = v1::IntermediateRewards::load_from_state(&state).unwrap(); + assert_eq!(BTreeMap::new(), final_rewards.current); + final_rewards.drain_calculated(); + assert_eq!(BTreeMap::new(), final_rewards.calculated); + } + #[test] fn candidates_deposit_add() { let mut state = helpers::get_temp_state(); diff --git a/core/src/consensus/stake/mod.rs b/core/src/consensus/stake/mod.rs index 02789499fe..5134367362 100644 --- a/core/src/consensus/stake/mod.rs +++ b/core/src/consensus/stake/mod.rs @@ -34,7 +34,7 @@ use primitives::{Bytes, H256}; use rlp::{Decodable, Rlp}; pub use self::action_data::{Banned, Validator, Validators}; -use self::action_data::{Candidates, Delegation, IntermediateRewards, Jail, ReleaseResult, StakeAccount, Stakeholders}; +use self::action_data::{Candidates, Delegation, Jail, ReleaseResult, StakeAccount, Stakeholders}; pub use self::actions::Action; pub use self::distribute::fee_distribute; use super::ValidatorSet; @@ -321,24 +321,61 @@ pub fn get_validators(state: &TopLevelState) -> StateResult { Validators::load_from_state(state) } -pub fn add_intermediate_rewards(state: &mut TopLevelState, address: Address, reward: u64) -> StateResult<()> { - let mut rewards = IntermediateRewards::load_from_state(state)?; - rewards.add_quantity(address, reward); - rewards.save_to_state(state)?; - Ok(()) -} +pub mod v0 { + use super::action_data::v0::IntermediateRewards; + use super::*; + + pub fn add_intermediate_rewards(state: &mut TopLevelState, address: Address, reward: u64) -> StateResult<()> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + rewards.add_quantity(address, reward); + rewards.save_to_state(state)?; + Ok(()) + } + + pub fn drain_previous_rewards(state: &mut TopLevelState) -> StateResult> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + let drained = rewards.drain_previous(); + rewards.save_to_state(state)?; + Ok(drained) + } -pub fn drain_previous_rewards(state: &mut TopLevelState) -> StateResult> { - let mut rewards = IntermediateRewards::load_from_state(state)?; - let drained = rewards.drain_previous(); - rewards.save_to_state(state)?; - Ok(drained) + pub fn move_current_to_previous_intermediate_rewards(state: &mut TopLevelState) -> StateResult<()> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + rewards.move_current_to_previous(); + rewards.save_to_state(state) + } } -pub fn move_current_to_previous_intermediate_rewards(state: &mut TopLevelState) -> StateResult<()> { - let mut rewards = IntermediateRewards::load_from_state(state)?; - rewards.move_current_to_previous(); - rewards.save_to_state(state) +pub mod v1 { + use super::action_data::v1::IntermediateRewards; + use super::*; + + pub fn add_intermediate_rewards(state: &mut TopLevelState, address: Address, reward: u64) -> StateResult<()> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + rewards.add_quantity(address, reward); + rewards.save_to_state(state)?; + Ok(()) + } + + pub fn drain_current_rewards(state: &mut TopLevelState) -> StateResult> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + let drained = rewards.drain_current(); + rewards.save_to_state(state)?; + Ok(drained) + } + + pub fn update_calculated_rewards(state: &mut TopLevelState, values: HashMap) -> StateResult<()> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + rewards.update_calculated(values.into_iter().collect()); + rewards.save_to_state(state) + } + + pub fn drain_calculated_rewards(state: &mut TopLevelState) -> StateResult> { + let mut rewards = IntermediateRewards::load_from_state(state)?; + let drained = rewards.drain_calculated(); + rewards.save_to_state(state)?; + Ok(drained) + } } pub fn update_validator_weights(state: &mut TopLevelState, block_author: &Address) -> StateResult<()> { diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 9bfc17b4a4..6644b1d50f 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -136,9 +136,43 @@ impl ConsensusEngine for Tendermint { /// Block transformation functions, before the transactions. fn on_open_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { + let client = self.client().ok_or(EngineError::CannotOpenBlock)?; + + let block_number = block.header().number(); let metadata = block.state().metadata()?.expect("Metadata must exist"); - if block.header().number() == metadata.last_term_finished_block_num() + 1 { - // FIXME: on_term_open + let era = metadata.term_params().map_or(0, |p| p.era()); + if block_number == metadata.last_term_finished_block_num() + 1 { + match era { + 0 => {} + 1 => { + let rewards = stake::v1::drain_current_rewards(block.state_mut())?; + let start_of_the_current_term = block_number; + let start_of_the_previous_term = { + let end_of_the_two_level_previous_term = client + .last_term_finished_block_num((metadata.last_term_finished_block_num() - 1).into()) + .unwrap(); + + end_of_the_two_level_previous_term + 1 + }; + + let banned = stake::Banned::load_from_state(block.state())?; + let start_of_the_current_term_header = + encoded::Header::new(block.header().clone().rlp_bytes().to_vec()); + + let pending_rewards = calculate_pending_rewards_of_the_previous_term( + &*client, + &*self.validators, + rewards, + start_of_the_current_term, + start_of_the_current_term_header, + start_of_the_previous_term, + &banned, + )?; + + stake::v1::update_calculated_rewards(block.state_mut(), pending_rewards)?; + } + _ => unimplemented!(), + } } Ok(()) } @@ -174,6 +208,7 @@ impl ConsensusEngine for Tendermint { let block_author_reward = total_reward - total_min_fee + distributor.remaining_fee(); + let era = term_common_params.map_or(0, |p| p.era()); let metadata = block.state().metadata()?.expect("Metadata must exist"); let term = metadata.current_term_id(); let term_seconds = match term { @@ -187,7 +222,11 @@ impl ConsensusEngine for Tendermint { } _ => { stake::update_validator_weights(block.state_mut(), &author)?; - stake::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?; + match era { + 0 => stake::v0::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?, + 1 => stake::v1::add_intermediate_rewards(block.state_mut(), author, block_author_reward)?, + _ => unimplemented!(), + } } } @@ -195,10 +234,10 @@ impl ConsensusEngine for Tendermint { return Ok(()) } - let inactive_validators = match term { - 0 => Vec::new(), - _ => { - let rewards = stake::drain_previous_rewards(block.state_mut())?; + let inactive_validators = match (era, term) { + (0, 0) => Vec::new(), + (0, _) => { + let rewards = stake::v0::drain_previous_rewards(block.state_mut())?; let start_of_the_current_term = metadata.last_term_finished_block_num() + 1; if term > 1 { @@ -232,7 +271,7 @@ impl ConsensusEngine for Tendermint { } } - stake::move_current_to_previous_intermediate_rewards(block.state_mut())?; + stake::v0::move_current_to_previous_intermediate_rewards(block.state_mut())?; let validators = stake::Validators::load_from_state(block.state())? .into_iter() @@ -240,13 +279,26 @@ impl ConsensusEngine for Tendermint { .collect(); inactive_validators(&*client, start_of_the_current_term, block.header(), validators) } + (1, _) => { + for (address, reward) in stake::v1::drain_calculated_rewards(block.state_mut())? { + self.machine.add_balance(block, &address, reward)?; + } + + let start_of_the_current_term = metadata.last_term_finished_block_num() + 1; + let validators = stake::Validators::load_from_state(block.state())? + .into_iter() + .map(|val| public_to_address(val.pubkey())) + .collect(); + inactive_validators(&*client, start_of_the_current_term, block.header(), validators) + } + _ => unimplemented!(), }; stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?; match term { 0 => {} - _ => match term_common_params.expect("Term common params should exist").era() { + _ => match era { 0 => {} 1 => block.state_mut().snapshot_term_params()?, _ => unimplemented!("It is not decided how we handle this"), From b172e19229d4094840fe8d7add039218d69dcd38 Mon Sep 17 00:00:00 2001 From: Joonmo Yang Date: Tue, 3 Dec 2019 15:52:42 +0900 Subject: [PATCH 20/20] Update the era with the value in current block's state At `on_term_close`, the term_common_params is not updated to the new parameters yet. But the parameters in the current block's state is updated, so we should get the `era` from there. --- core/src/consensus/tendermint/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/consensus/tendermint/engine.rs b/core/src/consensus/tendermint/engine.rs index 6644b1d50f..11023029ef 100644 --- a/core/src/consensus/tendermint/engine.rs +++ b/core/src/consensus/tendermint/engine.rs @@ -298,7 +298,7 @@ impl ConsensusEngine for Tendermint { match term { 0 => {} - _ => match era { + _ => match metadata.params().map_or(0, |p| p.era()) { 0 => {} 1 => block.state_mut().snapshot_term_params()?, _ => unimplemented!("It is not decided how we handle this"),