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

TimestampOverflow error on AuRa chain with multiple step durations #11895

Open
ngotchac opened this issue Oct 21, 2020 · 0 comments · May be fixed by #11896
Open

TimestampOverflow error on AuRa chain with multiple step durations #11895

ngotchac opened this issue Oct 21, 2020 · 0 comments · May be fixed by #11896

Comments

@ngotchac
Copy link
Contributor

Actual Issue

When using a chain configured with the AuRa engine, if multiple step durations are used, a TimestampOverflow error can shutdown the node (with a panic) if a block is detected as a future block.

Reproduction

To reproduce this issue, one simply needs to setup an AuRa chain, with a step duration transition, and put the clock of one of the validator a few seconds back, so that the next block appears as from the future.

Culprit

This comes from the check_future method which might return this error:

let d = self.durations.iter().take_while(|info| info.transition_step <= current).last()
	.expect("Duration map has at least a 0 entry.")
	.step_duration;
Err(Some(OutOfBounds {
	min: None,
	max: Some(d * current),
	found: d * given,
}))

The error is used in the verify_timestamp method:

let found = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(oob.found))

This comes from the old step definition (before introducing transitions in step duration):

let step = (now / step_duration);

which is now

let step = (now.checked_sub(transition_timestamp)? / step_duration).checked_add(transition_step)?; 

Invalid use of CheckedSystemTime::checked_add

I think there are also 2 invalid uses of CheckedSystemTime::checked_add here:

let min = CheckedSystemTime::checked_add(now, Duration::from_secs(parent.timestamp().saturating_add(1)))
	.ok_or(BlockError::TimestampOverflow)?;
let found = CheckedSystemTime::checked_add(now, Duration::from_secs(header.timestamp()))
	.ok_or(BlockError::TimestampOverflow)?;

whereas I think it should be CheckedSystemTime::checked_add(UNIX_EPOCH, ...). I didn't find any other incorrect uses.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant