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

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?

@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

3 participants