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

Pending Attestations RPC Server/Client Implementation #1617

Merged
merged 3 commits into from
Feb 16, 2019

Conversation

rauljordan
Copy link
Contributor

This is part of #1565 and a follow-up to #1606


Description

Write why you are making the changes in this pull request

In #1606, a method of obtaining pending attestations up to MAX_ATTESTATIONS from the operations service was implemented. This allows us to implement an RPC method in our server/client which can allow proposers to retrieve these pending attestations when performing their proposal responsibility.

Write a summary of the changes you are making

This PR implements a simple RPC server method called PendingAttestations, which returns a list of *pb.Attestation for a proposer to include in a beacon block. It also implements the call in the ProposerBlock method in validator/client/validator_propose.go. This PR adds relevant tests to ensure the behavior works as expected.

Link anything that would be helpful or relevant to the reviewers

Diff is large due to gomock.

@rauljordan rauljordan self-assigned this Feb 16, 2019
@rauljordan rauljordan added Ready For Review A pull request ready for code review Priority: High High priority item labels Feb 16, 2019
@rauljordan rauljordan added this to the Sapphire milestone Feb 16, 2019
@rauljordan rauljordan requested review from prestonvanloon, nisdas and terencechain and removed request for prestonvanloon and nisdas February 16, 2019 00:17
@codecov
Copy link

codecov bot commented Feb 16, 2019

Codecov Report

Merging #1617 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1617   +/-   ##
=======================================
  Coverage   71.35%   71.35%           
=======================================
  Files          94       94           
  Lines        6657     6657           
=======================================
  Hits         4750     4750           
  Misses       1500     1500           
  Partials      407      407

Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

LGTM

@rauljordan rauljordan merged commit c032b0f into master Feb 16, 2019
@rauljordan rauljordan deleted the integrate-att-pool branch February 16, 2019 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High High priority item Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants