Allow specifying stepDuration as a map.#152
Conversation
added a map variant to the stepDuration config parameter handling the initial step duration from the config map introduced the next step duration fixed step duration tests panic if step duration is 0 added panics on step duration 0 corrected the panic string moved step duration update to on_prepare_block corrected the use of the signer address moved the step duration update to on_close_block changed the step duration during the step increment added next_duration to tests calibrate the step timeout after duration update changing step duration seems to work updated tests estimating the start of a same step duration interval from time and the number of steps step durations derived from step numbers set the initial step to 0 review comments
| panic!("authority_round: step duration can't be zero") | ||
| if !our_params.step_durations.contains_key(&0) { | ||
| error!(target: "engine", "Authority Round step 0 duration is undefined, aborting"); | ||
| panic!("authority_round: step 0 duration is undefined") |
There was a problem hiding this comment.
Not sure; timestamp 0 is always also step 0, so this is the duration of step 0.
|
Should the timestamps be calculated in UTC or in the local time zone? |
afck
force-pushed
the
afck-step-duration-transition-rebased
branch
from
f8f946f to
b8ef2a4
Compare
|
UTC. The timestamps are in Unix time. |
varasev
left a comment
varasev
left a comment
There was a problem hiding this comment.
I wasn't able to run unit tests for the afck-step-duration-transition-rebased branch.
My rustc version is 1.34.0-nightly (d8a0dd7ae 2019-01-28).
When I try to perform cd ethcore && cargo test, I see the following errors:
| /// | ||
| /// Deliberately typed as u16 as too high of a value leads | ||
| /// to slow block issuance. | ||
| pub step_duration: u16, |
There was a problem hiding this comment.
We need to keep the step_duration parameter as well for backward compatibility.
There was a problem hiding this comment.
That should still be accepted: It can now either be a number, like before, or a map.
|
If I remove the redundant |
afck
force-pushed
the
afck-step-duration-transition-rebased
branch
from
b8ef2a4 to
5effbc7
Compare
|
Sorry! I updated the unit tests. |
3 participants
Closes #122, #124, #150.