feat: implement getStateRandao #6072
Conversation
|
|
|
@scorbajio thank you for your interest contributing! Please note that opening a PR and pushing commits triggers notifications to the entire team. I would recommend:
|
|
@dapplion I have to push back on this
While I think discord is great for many things like general questions and technical help, I don't think it is a great tool to collaborate and give code related feedback. It is also good to have all the discussion in one place, which I think a PR is perfect place for it, it also has resolvable threads per topic, and allows directly comment on the code which provides much more clarify.
I agree that it doesn't make sense to open a PR that has little to no changes that should be implemented but this is definitely not the case here. It good to open a PR asap in my opinion especially as a new contributor to get feedback on the direction of the changes and not waste time on things that wouldn't even work in the end. That's why draft exists and a PR is a great collaboration tool we should use as much as possible.
That's not a good reason why a PR should not be opened, there is always the option to unsubscribe, and that's it. We also had PRs open for weeks (or even months) and I think it is a good thing, it especially for new contributors to follow the work of others. Much better learning experience than just seeing the end result. |
There was a problem hiding this comment.
Thanks for looking into this @scorbajio! Some initial feedback / thoughts
| if (usedEpoch > stateEpoch) { | ||
| return ret; | ||
| } else if ( | ||
| usedEpoch < stateEpoch && | ||
| Math.abs(stateEpoch - epochsPerHistoricalVector) > 0 && | ||
| Math.abs(stateEpoch - epochsPerHistoricalVector) >= usedEpoch | ||
| ) { | ||
| return ret; | ||
| } |
There was a problem hiding this comment.
Not sure about this part, I think we should follow similar implementation as Lighthouse, seems simple and straight forward, see PR diff
As per spec randao.yaml:
400for invalid state id or epoch, mostly handled by json schema already (needs to be double checked)400if epoch is out of range, see spec note404if state not found
You can also check with other routes that load a state + have epoch as optional param for additional examples
There was a problem hiding this comment.
I was using teku's implementation as a reference. After taking a look at lighthouse, it seems like they do range checking in a more simple way in a separate get-index helper, which is what I used to rework the range-checking logic.
|
🎉 This PR is included in v1.12.0 🎉 |
Motivation
See #5701.
Description
The goal of this PR is to implement the get-state-randao api.