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

Batch Block Roots Requesting #7027

Merged
merged 14 commits into from Aug 20, 2020
Merged

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Aug 17, 2020

What type of PR is this?

Feature Change

What does this PR do? Why is it needed?

  • Adds ability to request multiple blocks from peers and allows local peer to retry randomly with other sets
    of peers.
  • Reduce dial timeouts to prevent being blocked by a lousy peer for too long.
  • Request only up to alloted limits and using the 'best' peers of a beacon node rather
    randomly trying out with any peer.

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas marked this pull request as ready for review August 18, 2020 18:46
@nisdas nisdas requested a review from a team as a code owner August 18, 2020 18:46
@nisdas nisdas added the Ready For Review A pull request ready for code review label Aug 18, 2020
@codecov
Copy link

codecov bot commented Aug 19, 2020

Codecov Report

Merging #7027 into master will increase coverage by 2.14%.
The diff coverage is 72.27%.

@@            Coverage Diff             @@
##           master    #7027      +/-   ##
==========================================
+ Coverage   60.07%   62.21%   +2.14%     
==========================================
  Files         323      401      +78     
  Lines       27422    31398    +3976     
==========================================
+ Hits        16473    19535    +3062     
- Misses       8733     9093     +360     
- Partials     2216     2770     +554     

farazdagi
farazdagi previously approved these changes Aug 19, 2020
@@ -49,6 +49,19 @@ func (s *Service) dedupBlocksAndRoots(blks []*ethpb.SignedBeaconBlock, roots [][
return newBlks, newRoots, nil
}

func (s *Service) dedupRoots(roots [][32]byte) [][32]byte {
Copy link
Member

Choose a reason for hiding this comment

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

Is this tested? I think we can add a duplicated root in TestService_BatchRootRequest to cover this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

great idea, will add that in

@prylabs-bulldozer prylabs-bulldozer bot merged commit 7744c3a into master Aug 20, 2020
@nisdas nisdas deleted the multipleBranchProcessing branch August 20, 2020 04:51
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.

None yet

3 participants