Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
fix: aura don't add SystemTime::now()
Browse files Browse the repository at this point in the history
In AURA we added `SystemTime::now` again which cases a Timestamp Overflow which this fixes
Also, because it is so annoying to the deal with `SystemTime` because it may be overflow when it is bigger than
`i32::max_value` then I changed the error to use `Duration` instead of `SystemTime`
  • Loading branch information
niklasad1 committed Jun 5, 2019
1 parent 4416187 commit cf0cda7
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 48 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion ethcore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ serde = "1.0"
serde_derive = "1.0"
stats = { path = "../util/stats" }
tempdir = { version = "0.3", optional = true }
time-utils = { path = "../util/time-utils" }
trace-time = "0.1"
triehash-ethereum = { version = "0.2", path = "../util/triehash-ethereum" }
unexpected = { path = "../util/unexpected" }
Expand Down
14 changes: 2 additions & 12 deletions ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ use types::header::{Header, ExtendedHeader};
use types::ancestry_action::AncestryAction;
use unexpected::{Mismatch, OutOfBounds};

#[cfg(not(time_checked_add))]
use time_utils::CheckedSystemTime;

mod finality;

/// `AuthorityRound` params.
Expand Down Expand Up @@ -583,15 +580,8 @@ fn verify_timestamp(step: &Step, header_step: u64) -> Result<(), BlockError> {
// NOTE This error might be returned only in early stage of verification (Stage 1).
// Returning it further won't recover the sync process.
trace!(target: "engine", "verify_timestamp: block too early");

let now = SystemTime::now();
let found = now.checked_add(Duration::from_secs(oob.found)).ok_or(BlockError::TimestampOverflow)?;
let max = oob.max.and_then(|m| now.checked_add(Duration::from_secs(m)));
let min = oob.min.and_then(|m| now.checked_add(Duration::from_secs(m)));

let new_oob = OutOfBounds { min, max, found };

Err(BlockError::TemporarilyInvalid(new_oob.into()))
let oob = oob.map(Duration::from_secs);
Err(BlockError::TemporarilyInvalid(oob.into()))
},
Ok(_) => Ok(()),
}
Expand Down
3 changes: 0 additions & 3 deletions ethcore/src/engines/clique/block_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ use types::BlockNumber;
use types::header::Header;
use unexpected::Mismatch;

#[cfg(not(feature = "time_checked_add"))]
use time_utils::CheckedSystemTime;

/// Type that keeps track of the state for a given vote
// Votes that go against the proposal aren't counted since it's equivalent to not voting
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
Expand Down
24 changes: 9 additions & 15 deletions ethcore/src/engines/clique/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ use unexpected::{Mismatch, OutOfBounds};
use types::BlockNumber;
use types::header::{ExtendedHeader, Header};

#[cfg(not(feature = "time_checked_add"))]
use time_utils::CheckedSystemTime;

use self::block_state::CliqueBlockState;
use self::params::CliqueParams;
use self::step_service::StepService;
Expand Down Expand Up @@ -546,17 +543,15 @@ impl Engine<EthereumMachine> for Clique {

// Don't waste time checking blocks from the future
{
let limit = SystemTime::now().checked_add(Duration::from_secs(self.period))
.ok_or(BlockError::TimestampOverflow)?;

let now = SystemTime::now();
let now = now.duration_since(UNIX_EPOCH).expect("now is after unix_epoch; qed");
// This should succeed under the contraints that the system clock works
let limit_as_dur = limit.duration_since(UNIX_EPOCH).map_err(|e| {
Box::new(format!("Converting SystemTime to Duration failed: {}", e))
})?;
// manually `saturating_add` for `Duration`
let limit = now.checked_add(Duration::from_secs(self.period))
.unwrap_or(Duration::from_secs(u64::max_value()));

let hdr = Duration::from_secs(header.timestamp());
if hdr > limit_as_dur {
let found = UNIX_EPOCH.checked_add(hdr).ok_or(BlockError::TimestampOverflow)?;
let found = Duration::from_secs(header.timestamp());
if found > limit {

Err(BlockError::TemporarilyInvalid(OutOfBounds {
min: None,
Expand Down Expand Up @@ -667,9 +662,8 @@ impl Engine<EthereumMachine> for Clique {
// Ensure that the block's timestamp isn't too close to it's parent
let limit = parent.timestamp().saturating_add(self.period);
if limit > header.timestamp() {
let max = UNIX_EPOCH.checked_add(Duration::from_secs(header.timestamp()));
let found = UNIX_EPOCH.checked_add(Duration::from_secs(limit))
.ok_or(BlockError::TimestampOverflow)?;
let max = Some(Duration::from_secs(header.timestamp()));
let found = Duration::from_secs(limit);

Err(BlockError::InvalidTimestamp(OutOfBounds {
min: None,
Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! General error types for use in ethcore.

use std::{fmt, error};
use std::time::SystemTime;
use std::time::Duration;

use derive_more::{Display, From};
use ethereum_types::{H256, U256, Address, Bloom};
Expand Down Expand Up @@ -134,12 +134,12 @@ pub enum BlockError {

/// Newtype for Display impl to show seconds
#[derive(Debug, Clone, From, PartialEq, Eq)]
pub struct OutOfBoundsTime(OutOfBounds<SystemTime>);
pub struct OutOfBoundsTime(OutOfBounds<Duration>);

impl fmt::Display for OutOfBoundsTime {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let seconds = self.0
.map(|st| st.elapsed().unwrap_or_default().as_secs());
.map(|st| st.as_secs());
f.write_fmt(format_args!("{}", seconds))
}
}
Expand Down
3 changes: 0 additions & 3 deletions ethcore/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,6 @@ extern crate fetch;
#[cfg(all(test, feature = "price-info"))]
extern crate parity_runtime;

#[cfg(not(time_checked_add))]
extern crate time_utils;

pub mod block;
pub mod builtin;
pub mod client;
Expand Down
20 changes: 10 additions & 10 deletions ethcore/src/verification/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ use types::{BlockNumber, header::Header};
use types::transaction::SignedTransaction;
use verification::queue::kind::blocks::Unverified;

#[cfg(not(time_checked_add))]
use time_utils::CheckedSystemTime;

/// Preprocessed block data gathered in `verify_block_unordered` call
pub struct PreverifiedBlock {
/// Populated block header
Expand Down Expand Up @@ -308,10 +305,13 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool,
if is_full {
const ACCEPTABLE_DRIFT: Duration = Duration::from_secs(15);
// this will resist overflow until `year 2037`
let max_time = SystemTime::now() + ACCEPTABLE_DRIFT;
let invalid_threshold = max_time + ACCEPTABLE_DRIFT * 9;
let timestamp = UNIX_EPOCH.checked_add(Duration::from_secs(header.timestamp()))
.ok_or(BlockError::TimestampOverflow)?;
let now = SystemTime::now();
let now = now.duration_since(UNIX_EPOCH).expect("now is after UNIX_EPOCH; qed");
let max_time = now.checked_add(ACCEPTABLE_DRIFT)
.unwrap_or(Duration::from_secs(u64::max_value()));
let invalid_threshold = max_time.checked_add(ACCEPTABLE_DRIFT * 9)
.unwrap_or(Duration::from_secs(u64::max_value()));
let timestamp = Duration::from_secs(header.timestamp());

if timestamp > invalid_threshold {
return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: Some(max_time), min: None, found: timestamp }.into())))
Expand All @@ -334,10 +334,10 @@ fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result

if !engine.is_timestamp_valid(header.timestamp(), parent.timestamp()) {
let now = SystemTime::now();
let now = now.duration_since(UNIX_EPOCH).expect("now is after UNIX_EPOCH; qed");
let min = now.checked_add(Duration::from_secs(parent.timestamp().saturating_add(1)))
.ok_or(BlockError::TimestampOverflow)?;
let found = now.checked_add(Duration::from_secs(header.timestamp()))
.ok_or(BlockError::TimestampOverflow)?;
.unwrap_or(Duration::from_secs(u64::max_value()));
let found = Duration::from_secs(header.timestamp());
return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: None, min: Some(min), found }.into())))
}
if header.number() != parent.number() + 1 {
Expand Down

0 comments on commit cf0cda7

Please sign in to comment.