Skip to content

Epoch: Fix the epoch update logic #164

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

Merged
merged 1 commit into from
Jul 2, 2020
Merged

Conversation

erikd
Copy link
Contributor

@erikd erikd commented Jul 2, 2020

No description provided.

This meets the requirements of:
* When syncing, the rows for old epochs are only updated once after the
  whole epoch has been retrieved.
* The point above is true even when the node is syncing (as opposed to
  following). Specifically, the TipBlock value provided by the node over
  the chain sync protocol is useless because it reports node tip, not
  network wide chain tip).
* Only updates the current epoch when we are close to the actual chain
  tip.

Closes: #111
Closes: #139
@tatyanavych tatyanavych requested a review from ksaric July 2, 2020 11:33
Copy link
Contributor

@ksaric ksaric left a comment

Choose a reason for hiding this comment

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

LGTM, even though I prefer my version with tests that show how specific conditions interact with the chain. As well as extra comments that explain what each branch is doing.
This is doing the same thing as the version I submitted, but it doesn't update the current and previous epoch, just the current one.

| epochNum > chainTipEpoch ->
-- Must just have started a new epoch, so call this which will
-- update chainTipEpoch.
| unSlotNo estTipSlot - unSlotNo tipSlot < 50 ->
Copy link
Contributor

Choose a reason for hiding this comment

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

If for some reason we have a "lag" of 50+ slots/blocks, the epoch number will not be updated until the next epoch.

Copy link
Contributor Author

@erikd erikd Jul 2, 2020

Choose a reason for hiding this comment

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

How could we have a "lag" of 50 slots? The protocol itself relies on exact times and on all block producers to agree on the same global time.

@disassembler
Copy link
Contributor

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 2, 2020

@iohk-bors iohk-bors bot merged commit f23bd79 into master Jul 2, 2020
@iohk-bors iohk-bors bot deleted the erikd/epoch-table-update branch July 2, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants