fix: add slot and epoch validation to epoch committees endpoint#7941
Conversation
|
|
|
Hello @nflaig Could you please review this when you have time? |
|
@bomanaps are you planning to continue working on this? it resolves an error we see on the devnet as well and would be good to get this in soon |
| if (error instanceof Error && error.message?.includes("EPOCH_CONTEXT_ERROR_DECISION_ROOT_EPOCH_OUT_OF_RANGE")) { | ||
| throw new ApiError(400, `Invalid epoch value: epoch ${epoch} is out of range`); | ||
| } |
There was a problem hiding this comment.
as noted in #7941 (comment) I don't see why this is necessary, we check the epoch above to make sure it's within one epoch of state epoch
| const stateEpoch = computeEpochAtSlot(state.slot); | ||
| if (filters.slot !== undefined) { | ||
| const epochStartSlot = computeStartSlotAtEpoch(epoch); | ||
| const epochEndSlot = epochStartSlot + 32 - 1; |
There was a problem hiding this comment.
this will break with minimal preset as an epoch only has 8 slots, always need to use SLOTS_PER_EPOCH
| } | ||
| } | ||
| if (Math.abs(epoch - stateEpoch) > 1) { | ||
| throw new ApiError(400, `Invalid epoch value: epoch ${epoch} is out of range`); |
There was a problem hiding this comment.
would be useful to return state epoch and explain in the error that the epoch needs to be within one epoch, "out of range" doesn't provide enough details imo
nflaig
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## unstable #7941 +/- ##
============================================
- Coverage 54.11% 54.11% -0.01%
============================================
Files 849 849
Lines 64043 64051 +8
Branches 4854 4855 +1
============================================
+ Hits 34659 34662 +3
- Misses 29307 29312 +5
Partials 77 77 🚀 New features to boost your workflow:
|
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds validation to the /eth/v1/beacon/states/{state_id}/committees endpoint to handle cases where slot and epoch query parameters are inconsistent. The changes correctly introduce two checks: one to ensure a provided slot falls within the specified epoch, and another to ensure the requested epoch is within one epoch of the state's epoch. This prevents an unhandled error that previously resulted in a 500 response. The implementation is correct and effectively resolves the issue. I've suggested a minor reordering of the validation checks to improve logical flow by failing earlier on more fundamental constraints.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
🎉 This PR is included in v1.34.0 🎉 |
4 participants
Motivation
Description
This PR fixes an inconsistency in Lodestar's Beacon-API behavior when querying:
Previously, Lodestar would return a
500 Internal Server Errorwhen the provided slot did not belong to the given epoch (e.g.,epoch=2&slot=118), due to an unhandledEPOCH_CONTEXT_ERROR_DECISION_ROOT_EPOCH_OUT_OF_RANGEerror.Closes #7882
Steps to test or reproduce