Skip to content

Do not prevote to the block with irrelevant generation time #1512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions codechain/codechain.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@ version_short: "v"
author: CodeChain Team <[email protected]>
about: CodeChian client
args:
- allowed-future-gap:
long: allowed-future-gap
value_name: MS
help: Specify the allowed gap in the future direction from the system time to the block generation time. MS is time measured in milliseconds.
takes_value: true
- allowed-past-gap:
long: allowed-past-gap
value_name: MS
help: Specify the allowed gap in the past direction from the system time to the block generation time. MS is time measured in milliseconds.
takes_value: true
- config:
long: config
help: Specify the certain config file path that you want to use to configure CodeChain to your needs.
Expand Down
20 changes: 19 additions & 1 deletion codechain/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::fs;
use std::str::{self, FromStr};
use std::time::Duration;

use ccore::{MinerOptions, StratumConfig};
use ccore::{MinerOptions, StratumConfig, TimeGapParams};
use cidr::IpCidr;
use ckey::PlatformAddress;
use clap;
Expand Down Expand Up @@ -226,6 +226,8 @@ pub struct Mining {
pub reseal_max_period: Option<u64>,
pub no_reseal_timer: Option<bool>,
pub work_queue_size: Option<usize>,
pub allowed_past_gap: Option<u64>,
pub allowed_future_gap: Option<u64>,
}

#[derive(Deserialize)]
Expand Down Expand Up @@ -446,8 +448,24 @@ impl Mining {
if let Some(work_queue_size) = matches.value_of("work-queue-size") {
self.work_queue_size = Some(work_queue_size.parse().map_err(|_| "Invalid size")?);
}
if let Some(allowed_past_gap) = matches.value_of("allowed-past-gap") {
self.allowed_past_gap = Some(allowed_past_gap.parse().map_err(|_| "Invalid time gap")?);
}
if let Some(allowed_future_gap) = matches.value_of("allowed-future-gap") {
self.allowed_future_gap = Some(allowed_future_gap.parse().map_err(|_| "Invalid time gap")?);
}
Ok(())
}

pub fn create_time_gaps(&self) -> TimeGapParams {
let allowed_past_gap = Duration::from_millis(self.allowed_past_gap.unwrap_or(30000));
let allowed_future_gap = Duration::from_millis(self.allowed_future_gap.unwrap_or(5000));

TimeGapParams {
allowed_past_gap,
allowed_future_gap,
}
}
}

impl Network {
Expand Down
2 changes: 2 additions & 0 deletions codechain/config/presets/config.dev.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ reseal_min_period = 0
reseal_max_period = 120000
no_reseal_timer = false
work_queue_size = 20
allowed_past_gap = 30000
allowed_future_gap = 5000

[network]
disable = false
Expand Down
2 changes: 2 additions & 0 deletions codechain/config/presets/config.prod.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ reseal_min_period = 4000
reseal_max_period = 120000
no_reseal_timer = false
work_queue_size = 20
allowed_past_gap = 30000
allowed_future_gap = 5000

[network]
disable = false
Expand Down
2 changes: 2 additions & 0 deletions codechain/run_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,12 @@ pub fn run_node(matches: &ArgMatches) -> Result<(), String> {

let config = load_config(matches)?;

let time_gap_params = config.mining.create_time_gaps();
let scheme = match &config.operating.chain {
Some(chain) => chain.scheme()?,
None => return Err("chain is not specified".to_string()),
};
scheme.engine.register_time_gap_config_to_worker(time_gap_params);

let instance_id = config.operating.instance_id.unwrap_or(
SystemTime::now()
Expand Down
4 changes: 3 additions & 1 deletion core/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub use self::cuckoo::Cuckoo;
pub use self::null_engine::NullEngine;
pub use self::simple_poa::SimplePoA;
pub use self::solo::Solo;
pub use self::tendermint::{Tendermint, TendermintParams};
pub use self::tendermint::{Tendermint, TendermintParams, TimeGapParams};
pub use self::validator_set::validator_list::ValidatorList;
pub use self::validator_set::ValidatorSet;

Expand Down Expand Up @@ -255,6 +255,8 @@ pub trait ConsensusEngine<M: Machine>: Sync + Send {

fn register_network_extension_to_service(&self, _: &NetworkService) {}

fn register_time_gap_config_to_worker(&self, _time_gap_params: TimeGapParams) {}

fn score_to_target(&self, _score: &U256) -> U256 {
U256::zero()
}
Expand Down
5 changes: 5 additions & 0 deletions core/src/consensus/tendermint/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::consensus::EngineType;
use crate::error::Error;
use crate::header::Header;
use crate::views::HeaderView;
use consensus::tendermint::params::TimeGapParams;

impl ConsensusEngine<CodeChainMachine> for Tendermint {
fn name(&self) -> &str {
Expand Down Expand Up @@ -226,6 +227,10 @@ impl ConsensusEngine<CodeChainMachine> for Tendermint {
receiver.recv().unwrap();
}

fn register_time_gap_config_to_worker(&self, time_gap_params: TimeGapParams) {
self.external_params_initializer.send(time_gap_params).unwrap();
}

fn block_reward(&self, _block_number: u64) -> u64 {
self.block_reward
}
Expand Down
13 changes: 11 additions & 2 deletions core/src/consensus/tendermint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use parking_lot::RwLock;
use primitives::H256;

use self::chain_notify::TendermintChainNotify;
pub use self::params::{TendermintParams, TimeoutParams};
pub use self::params::{TendermintParams, TimeGapParams, TimeoutParams};
use self::types::{Height, Step, View};
use super::stake;
use crate::client::EngineClient;
Expand All @@ -56,6 +56,7 @@ pub type BlockHash = H256;
/// ConsensusEngine using `Tendermint` consensus algorithm
pub struct Tendermint {
client: RwLock<Option<Weak<EngineClient>>>,
external_params_initializer: crossbeam::Sender<TimeGapParams>,
extension_initializer: crossbeam::Sender<(crossbeam::Sender<network::Event>, Weak<EngineClient>)>,
timeouts: TimeoutParams,
join: Option<JoinHandle<()>>,
Expand Down Expand Up @@ -89,12 +90,14 @@ impl Tendermint {
let timeouts = our_params.timeouts;
let machine = Arc::new(machine);

let (join, extension_initializer, inner, quit_tendermint) = worker::spawn(our_params.validators);
let (join, external_params_initializer, extension_initializer, inner, quit_tendermint) =
worker::spawn(our_params.validators);
let action_handlers: Vec<Arc<ActionHandler>> = vec![Arc::new(stake)];
let chain_notify = Arc::new(TendermintChainNotify::new(inner.clone()));

Arc::new(Tendermint {
client: Default::default(),
external_params_initializer,
extension_initializer,
timeouts,
join: Some(join),
Expand Down Expand Up @@ -170,7 +173,13 @@ mod tests {

#[test]
fn has_valid_metadata() {
use std::time::Duration;
let engine = Scheme::new_test_tendermint().engine;
let time_gap_params = TimeGapParams {
allowed_past_gap: Duration::from_millis(30000),
allowed_future_gap: Duration::from_millis(5000),
};
engine.register_time_gap_config_to_worker(time_gap_params);
assert!(!engine.name().is_empty());
}

Expand Down
5 changes: 5 additions & 0 deletions core/src/consensus/tendermint/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ fn to_duration(ms: cjson::uint::Uint) -> Duration {
Duration::from_millis(ms as u64)
}

pub struct TimeGapParams {
pub allowed_past_gap: Duration,
pub allowed_future_gap: Duration,
}

/// Base timeout of each step in ms.
#[derive(Debug, Copy, Clone)]
pub struct TimeoutParams {
Expand Down
48 changes: 42 additions & 6 deletions core/src/consensus/tendermint/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::iter::Iterator;
use std::mem;
use std::sync::{Arc, Weak};
use std::thread::{Builder, JoinHandle};
use std::time::{Duration, SystemTime, UNIX_EPOCH};

use ccrypto::blake256;
use ckey::{public_to_address, verify_schnorr, Address, SchnorrSignature};
Expand All @@ -31,6 +32,7 @@ use rlp::{Encodable, UntrustedRlp};
use super::backup::{backup, restore, BackupView};
use super::message::*;
use super::network;
use super::params::TimeGapParams;
use super::types::{BitSet, Height, Proposal, Step, TendermintSealView, TendermintState, TwoThirdsMajority, View};
use super::{
BlockHash, ENGINE_TIMEOUT_BROADCAST_STEP_STATE, ENGINE_TIMEOUT_EMPTY_PROPOSAL, ENGINE_TIMEOUT_TOKEN_NONCE_BASE,
Expand All @@ -51,6 +53,7 @@ use crate::BlockId;

type SpawnResult = (
JoinHandle<()>,
crossbeam::Sender<TimeGapParams>,
crossbeam::Sender<(crossbeam::Sender<network::Event>, Weak<EngineClient>)>,
crossbeam::Sender<Event>,
crossbeam::Sender<()>,
Expand Down Expand Up @@ -86,7 +89,7 @@ struct Worker {
validators: Arc<ValidatorSet>,
/// Channel to the network extension, must be set later.
extension: EventSender<network::Event>,

time_gap_params: TimeGapParams,
timeout_token_nonce: usize,
}

Expand Down Expand Up @@ -166,7 +169,12 @@ pub enum Event {
impl Worker {
#![cfg_attr(feature = "cargo-clippy", allow(clippy::new_ret_no_self))]
/// Create a new instance of Tendermint engine
fn new(validators: Arc<ValidatorSet>, extension: EventSender<network::Event>, client: Weak<EngineClient>) -> Self {
fn new(
validators: Arc<ValidatorSet>,
extension: EventSender<network::Event>,
client: Weak<EngineClient>,
time_gap_params: TimeGapParams,
) -> Self {
Worker {
client,
height: 1,
Expand All @@ -181,18 +189,26 @@ impl Worker {
extension,
votes_received: BitSet::new(),
votes_received_changed: false,

time_gap_params,
timeout_token_nonce: ENGINE_TIMEOUT_TOKEN_NONCE_BASE,
}
}

fn spawn(validators: Arc<ValidatorSet>) -> SpawnResult {
let (sender, receiver) = crossbeam::unbounded();
let (quit, quit_receiver) = crossbeam::bounded(1);
let (external_params_initializer, external_params_receiver) = crossbeam::bounded(1);
let (extension_initializer, extension_receiver) = crossbeam::bounded(1);
let join = Builder::new()
.name("tendermint".to_string())
.spawn(move || {
let time_gap_params = match external_params_receiver.recv() {
Ok(time_gap_params) => time_gap_params,
Err(crossbeam::RecvError) => {
cerror!(ENGINE, "The tendermint external parameters are not initialized");
return
}
};
let (extension, client) = crossbeam::select! {
recv(extension_receiver) -> msg => {
match msg {
Expand All @@ -214,7 +230,7 @@ impl Worker {
}
};
validators.register_client(Weak::clone(&client));
let mut inner = Self::new(validators, extension, client);
let mut inner = Self::new(validators, extension, client, time_gap_params);
loop {
crossbeam::select! {
recv(receiver) -> msg => {
Expand Down Expand Up @@ -341,7 +357,7 @@ impl Worker {
}
})
.unwrap();
(join, extension_initializer, sender, quit)
(join, external_params_initializer, extension_initializer, sender, quit)
}

/// The client is a thread-safe struct. Using it in multi-threads is safe.
Expand Down Expand Up @@ -651,11 +667,16 @@ impl Worker {
// In the case, self.votes_received is not empty.
self.request_messages_to_all(vote_step, &BitSet::all_set() - &self.votes_received);
if !self.already_generated_message() {
let block_hash = match &self.last_two_thirds_majority {
let block_hash_candidate = match &self.last_two_thirds_majority {
TwoThirdsMajority::Empty => self.proposal.imported_block_hash(),
TwoThirdsMajority::Unlock(_) => self.proposal.imported_block_hash(),
TwoThirdsMajority::Lock(_, block_hash) => Some(*block_hash),
};
let block_hash = block_hash_candidate.filter(|hash| {
let block =
self.client().block(&BlockId::Hash(*hash)).expect("Already got imported block hash");
self.is_generation_time_relevant(&block.decode_header())
});
self.generate_and_broadcast_message(block_hash, is_restoring);
}
}
Expand Down Expand Up @@ -686,6 +707,21 @@ impl Worker {
}
}

fn is_generation_time_relevant(&self, block_header: &Header) -> bool {
let acceptable_past_gap = self.time_gap_params.allowed_past_gap;
let acceptable_future_gap = self.time_gap_params.allowed_future_gap;
let now = SystemTime::now();
let allowed_min = now - acceptable_past_gap;
let allowed_max = now + acceptable_future_gap;
let block_generation_time = UNIX_EPOCH.checked_add(Duration::from_secs(block_header.timestamp()));

match block_generation_time {
Some(generation_time) => generation_time <= allowed_max && allowed_min <= generation_time,
// Overflow occurred
None => false,
}
}

fn locked_proposal_block(&self, locked_view: View) -> Result<encoded::Block, String> {
let vote_step = VoteStep::new(self.height, locked_view, Step::Propose);
let locked_proposal_hash = self.votes.get_block_hashes(&vote_step).first().cloned();
Expand Down
2 changes: 1 addition & 1 deletion core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub use crate::client::{
EngineClient, EngineInfo, ExecuteClient, ImportBlock, MiningBlockChainClient, RegularKey, RegularKeyOwner, Seq,
Shard, StateInfo, TestBlockChainClient, TextClient,
};
pub use crate::consensus::EngineType;
pub use crate::consensus::{EngineType, TimeGapParams};
pub use crate::db::{COL_STATE, NUM_COLUMNS};
pub use crate::error::{BlockImportError, Error, ImportError};
pub use crate::header::{Header, Seal};
Expand Down
4 changes: 4 additions & 0 deletions json/src/scheme/tendermint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ pub struct TendermintParams {
pub block_reward: Option<Uint>,
/// How much tokens are distributed at Genesis?
pub genesis_stakes: Option<HashMap<PlatformAddress, u64>>,
/// allowed past time gap in milliseconds.
pub allowed_past_timegap: Option<Uint>,
/// allowed future time gap in milliseconds.
pub allowed_future_timegap: Option<Uint>,
}

/// Tendermint engine deserialization.
Expand Down