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

Support millisecond timestamp for instant seal engine #9469

Merged
merged 6 commits into from
Sep 6, 2018
Merged

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Sep 4, 2018

Fixes #9468

In instant seal engine, one can set the millisecondTimestamp config to enable millisecond timestamp resolution. I didn't add nanosecond support because that overflows u64.

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 4, 2018
@sorpaas sorpaas added this to the 2.1 milestone Sep 4, 2018
pub struct InstantSealParams {
/// Whether to enable millisecond timestamp.
#[serde(rename="millisecondTimestamp")]
pub millisecond_timestamp: Option<bool>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use #[serde(default)] and make it a bool? Option<bool> seems like an overkill.

Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just think we can change the type in the json spec as @ordian mentioned.

@5chdn 5chdn added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 6, 2018
@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Sep 6, 2018
@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 6, 2018
@ordian ordian merged commit 5752869 into master Sep 6, 2018
@ordian ordian deleted the sp-nanosecond branch September 6, 2018 09:38
@Tbaut
Copy link
Contributor

Tbaut commented Sep 11, 2018

Can we add this to the wiki please @sorpaas ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants