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

More efficient ancestor head retrieval for GetAttestationData #5669

Merged
merged 3 commits into from
Apr 29, 2020

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Apr 28, 2020

What type of PR is this?

Feature

What does this PR do? Why is it needed?

In the event of head slot > request slot, the node starts fetching ancestor head root recursively until the condition satisfies. Every slot is a DB look up.

Instead, we can use helpers.BlockRootAtSlot(headState, req.Slot) to limit the DB look up to just one time.

Which issues(s) does this PR fix?

Could be relevant to #5665

Other notes for review

Tested with my 3k validators with both enable-new-state-mgmt on and off

@terencechain terencechain added Enhancement New feature or request Ready For Review A pull request ready for code review labels Apr 28, 2020
@terencechain terencechain requested a review from a team as a code owner April 28, 2020 22:43
@terencechain terencechain self-assigned this Apr 28, 2020
@terencechain terencechain added Blocked Blocked by research or external factors and removed Ready For Review A pull request ready for code review labels Apr 28, 2020
@terencechain
Copy link
Member Author

Marking this as blocked to merge. I want to test it for few more hours

@codecov
Copy link

codecov bot commented Apr 28, 2020

Codecov Report

Merging #5669 into master will increase coverage by 5.03%.
The diff coverage is 30.76%.

@@            Coverage Diff             @@
##           master    #5669      +/-   ##
==========================================
+ Coverage   14.90%   19.93%   +5.03%     
==========================================
  Files         116      239     +123     
  Lines        9328    20825   +11497     
==========================================
+ Hits         1390     4152    +2762     
- Misses       7711    15878    +8167     
- Partials      227      795     +568     

@rauljordan rauljordan merged commit 5636cd3 into master Apr 29, 2020
@delete-merged-branch delete-merged-branch bot deleted the efficient-head-retrival branch April 29, 2020 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Blocked by research or external factors Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants