Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bugfix/geth console displays incorrect block timestamp #699

Closed
wants to merge 8 commits into from
Closed

Bugfix/geth console displays incorrect block timestamp #699

wants to merge 8 commits into from

Conversation

SatpalSandhu61
Copy link
Contributor

Fixes #698
Fix issue where geth console displays incorrect timestamp under Raft, due to Raft holding block timestamp in nanoseconds, instead of seconds.

@SatpalSandhu61
Fix issue where geth console displays incorrect timestamp under Raft,…
a577f1f 
… due to Raft holding block timestamp in nanoseconds, instead of seconds.
@SatpalSandhu61
Merge branch 'master' of https://github.com/jpmorganchase/quorum into…
469baf8 
… bugfix/geth-console-displays-incorrect-block-timestamp
@SatpalSandhu61 SatpalSandhu61 mentioned this pull request Apr 30, 2019
Closed
@jbhurat
Copy link
Contributor

jbhurat commented Apr 30, 2019

Hi @SatpalSandhu61, can you please run goimport on the modified go source file

@SatpalSandhu61
Copy link
Contributor Author

@jbhurat - ok done. I didn't realise Intellij was using it's own rules for import formatting, rather than the gofmt rules. I've installed the goformats plugin now.

jbhurat
jbhurat previously approved these changes Apr 30, 2019
View reviewed changes
fixanoid
fixanoid previously approved these changes May 1, 2019
View reviewed changes
jpmsam
jpmsam reviewed May 2, 2019
View reviewed changes
console/console.go Outdated

//If timestamp is greater than max possible value (in seconds) for Unix time, then assume it's nanoseconds.
//If we fail to parse the timestamp then just assume it is in seconds.
timestamp, err := strconv.ParseInt(lastBlock.Timestamp, 0, 64)
Copy link
Contributor

Choose a reason for hiding this comment

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

@SatpalSandhu61 is it possible to check here if Raft is configured ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See updated PR - I now use a new API method to determine whether Raft is configured.

@SatpalSandhu61
Added a new API method GetNodeConfig(), which is used for more reliab…
9c1e6fc 
…le indication of the consensus mechanism in use.
@SatpalSandhu61
Merge remote-tracking branch 'origin/bugfix/geth-console-displays-inc…
3da9bfa 
…orrect-block-timestamp' into bugfix/geth-console-displays-incorrect-block-timestamp
@SatpalSandhu61 SatpalSandhu61 dismissed stale reviews from fixanoid and jbhurat via 3da9bfa May 30, 2019 12:42
@trung
Copy link
Contributor

trung commented Jun 11, 2019

There's an existing admin_nodeInfo API. Can we put additional info in here rather than creating a new one?

@SatpalSandhu61
Copy link
Contributor Author

There's an existing admin_nodeInfo API. Can we put additional info in here rather than creating a new one?

That's a good point. I'll take a look.

@SatpalSandhu61
Copy link
Contributor Author

Closing PR, will raise another one with changes.

@SatpalSandhu61 SatpalSandhu61 deleted the bugfix/geth-console-displays-incorrect-block-timestamp branch June 21, 2019 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpmsam jpmsam jpmsam left review comments

@jbhurat jbhurat jbhurat left review comments

@fixanoid fixanoid fixanoid left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Raft consensus: geth console displays incorrect block timestamp
5 participants
@SatpalSandhu61 @jbhurat @trung @fixanoid @jpmsam