Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Updated Average Block Time #548

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

MonikaCat
Copy link
Contributor

@MonikaCat MonikaCat commented Jul 15, 2021

Description

Fixes #542

Checklist

  • Targeted PR against correct branch.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Run meteor npm run lint
  • Added an entry to the CHANGELOG.md file.
  • Re-reviewed Files changed in the Github PR explorer.

@MonikaCat MonikaCat marked this pull request as ready for review July 15, 2021 10:37
@MonikaCat MonikaCat requested a review from kwunyeung July 15, 2021 10:37
timeDiff = Math.abs(dateLatest.getTime() - dateLast.getTime());
// blockTime = (chainStatus.blockTime * (blockData.height - 1) + timeDiff) / blockData.height;
blockTime = (dateLatest.getTime() - genesisTime.getTime()) / blockData.height;
blockTime = (height - blockDiff > 0) ? ((dateLatest.getTime() - pastBlockTime.getTime()) / blockDiff) : (dateLatest.getTime() - pastBlockTime.getTime());
Copy link
Member

Choose a reason for hiding this comment

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

@MonikaCat can you explain the whole logic here?

In the original implementation, averageBlcokTime = lastBlockTime - genesisTime / blockHeight. This is always true for the idea of average block time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the genesis time value had been removed from the settings.json file, we cannot calculate the average block time by using averageBlockTime = lastBlockTime - genesisTime / blockHeight. To handle the calculations, I initially added averageBlockTimeWindow in settings.json file to calculate the average block time of last n blocks...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, I changed the approach and I've just updated the code to calculate average block time starting from the startHeight height set in params in settings.json file

Copy link
Member

Choose a reason for hiding this comment

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

Ah... I have missed the genesisTime in the default_settings.json. It should be there as this is how the average block time is being calculated. Querying from chain is not feasible as the genesis could be huge. The size on cosmoshub-4 can't even give a full response if we query from the api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see! Alright I will update the code to include genesisTime calculations again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, how does it do now?

imports/ui/home/ChainStatus.jsx Outdated Show resolved Hide resolved
@MonikaCat MonikaCat requested a review from kwunyeung August 6, 2021 14:34
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 this pull request may close these issues.

Average block time calculation neglects startHeight
2 participants