-
Notifications
You must be signed in to change notification settings - Fork 65
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 Debug Forkchoice endpoint support #49
Conversation
This endpoint doesn't seem to be very well supported:
so 1/4 of the nodes on which I tried it. I'm a little concerned about adding support for this in to the codebase and then being pinged on issues using it due to the lack of downstream support. Are there plans for the various clients to add/fix support? |
Yeah that's one of my concerns as well. Maybe it makes sense to rename the func to Teku: Implemented |
Happy for this to stay unmerged until more support lands in client releases @mcdee FWIW 🙏 |
Yeah let's hold this until we see better coverage of support and can take a view on where it fits. |
@mcdee All clients excluding Lodestar have support in at least their |
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.
One change required. Also, please could you add this call to the sleepy
and erroring
clients in the testclients
directory? Thanks.
There are a few errors being thrown up by the testing, please could you rebase this against he latest master and we'll see what needs to be addressed? Thanks. |
Not sure exactly where those errors are (the output of the testing looks bad), I'll merge this and fix if required in a separate commit. |
Cheers @mcdee! Sorry I've been on holidays for a little bit, thank you for sorting that 🙏 |
Adds support for the new ForkChoice debug endpoint.
I wasn't entirely sure on the structure/formatting of where to place these structs. Happy to be advised if there is a better way to do things 🙏
Thank you!