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.

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

Offset in blocks retrieval for average block time #2902

Merged
merged 2 commits into from
Dec 6, 2019

Conversation

vbaranov
Copy link
Member

@vbaranov vbaranov commented Dec 2, 2019

Motivation

  1. If non-consensus blocks are excluded in the calculation, the total amount of accounted blocks is <= 100. But it should be 100 for both cases: excluding/including of non-consensus blocks.
  2. function refresh_timestamps gets the latest blocks but there could be reorgs and thus missing blocks in the DB that contaminates the average value.
    For instance, on Kovan it should be 4 sec:

Screenshot 2019-12-02 at 14 53 59

Changelog

  • add an offset of 100 blocks in refresh_timestamps function
  • fixate limit of 100 target blocks for the case, when we exclude non-consensus blocks from calculation

Checklist for your Pull Request (PR)

@vbaranov vbaranov self-assigned this Dec 2, 2019
@vbaranov vbaranov force-pushed the vb-average-time-calculation-offset branch from 349d02a to 477676b Compare December 2, 2019 11:58
@vbaranov vbaranov added the ready for review This PR is ready for reviews. label Dec 2, 2019
@vbaranov vbaranov force-pushed the vb-average-time-calculation-offset branch from 477676b to 86068a2 Compare December 2, 2019 12:02
@coveralls
Copy link

coveralls commented Dec 2, 2019

Pull Request Test Coverage Report for Build 48f53e8c-d217-45fc-bb9f-41b5abdcec43

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 75.018%

Totals Coverage Status
Change from base Build 21dd49e8-0ec0-4b2b-a14e-88a97f0ee0c7: -0.004%
Covered Lines: 5276
Relevant Lines: 7033

💛 - Coveralls

@vbaranov vbaranov force-pushed the vb-average-time-calculation-offset branch 7 times, most recently from b7efd69 to 88dfdf2 Compare December 2, 2019 16:33
@vbaranov vbaranov force-pushed the vb-average-time-calculation-offset branch from 88dfdf2 to 38f2e0f Compare December 2, 2019 17:01
@vbaranov vbaranov merged commit bd7b367 into master Dec 6, 2019
@vbaranov vbaranov deleted the vb-average-time-calculation-offset branch December 6, 2019 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for reviews.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants