-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add REST implementation for Validator's WaitForChainStart #11654
Add REST implementation for Validator's WaitForChainStart #11654
Conversation
…ithub.com:PatriceVignola/prysm into user/pavignol/add-wait-for-chain-start-beacon-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! But the new waitForChainStart
function doesn't have any tests. I think you should be able to write some with the help of https://pkg.go.dev/net/http/httptest for mocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add more tests for the new functionality
…ithub.com:PatriceVignola/prysm into user/pavignol/add-wait-for-chain-start-beacon-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
"github.com/prysmaticlabs/prysm/v3/testing/assert" | ||
) | ||
|
||
func TestBeaconApiHelpers(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! In my opinion, this is so much cleaner.
What type of PR is this?
Feature
What does this PR do? Why is it needed?
This adds an implementation for the
WaitForChainStart
validator client method by caling the Beacon Node REST API instead of the gRPC one.