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

Handle missing archived index #5899

Merged
merged 9 commits into from May 18, 2020
Merged

Conversation

terencechain
Copy link
Member

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?
When an archived slot has a skip block, the node should be looking back to last archived slot to get a real root instead of returning 0x0000...This also handles genesis case instead of returning 0x0000...

Which issues(s) does this PR fix?

Fixes #5898

Other notes for review
Tested locally. Was able to request previously failed historical states via debug end point

@terencechain terencechain added the Ready For Review A pull request ready for code review label May 18, 2020
@terencechain terencechain requested a review from a team as a code owner May 18, 2020 16:55
@terencechain terencechain self-assigned this May 18, 2020
archivedIndex := uint64(0)
if slot/params.BeaconConfig().SlotsPerArchivedPoint > 1 {
archivedIndex = slot/params.BeaconConfig().SlotsPerArchivedPoint - 1
}
return s.beaconDB.ArchivedPointRoot(ctx, archivedIndex)
fmt.Println(archivedIndex)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this number increasing constantly, reached 130 and I turned off my slasher.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's good. That means your slasher is doing work 👍 I removed the debug log

0xKiwi
0xKiwi previously approved these changes May 18, 2020
if s.beaconDB.HasArchivedPoint(ctx, archivedIndex) {
return s.beaconDB.ArchivedPointRoot(ctx, archivedIndex), nil
}
archivedIndex--
Copy link
Member

Choose a reason for hiding this comment

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

What is this? Can you add more to the godoc description about what this implementation is doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

will do

@codecov
Copy link

codecov bot commented May 18, 2020

Codecov Report

Merging #5899 into master will increase coverage by 0.30%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5899      +/-   ##
==========================================
+ Coverage   59.83%   60.13%   +0.30%     
==========================================
  Files         314      314              
  Lines       26505    26431      -74     
==========================================
+ Hits        15858    15895      +37     
+ Misses       8486     8391      -95     
+ Partials     2161     2145      -16     

@prylabs-bulldozer prylabs-bulldozer bot merged commit 678347e into master May 18, 2020
@delete-merged-branch delete-merged-branch bot deleted the handle-missing-archived-index branch May 18, 2020 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unknown block when requesting historical state
3 participants