-
Notifications
You must be signed in to change notification settings - Fork 792
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
[Merged by Bors] - Fix attestation performance API InvalidValidatorIndex
error
#3503
Conversation
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 looks good.
I'm wondering whether we should also update the comment here to reflect that these functions do not in fact return false
when the index is out of bounds, and may return an error:
lighthouse/consensus/state_processing/src/per_epoch_processing/epoch_processing_summary.rs
Lines 211 to 214 in 18c61a5
/// ## Notes | |
/// | |
/// Always returns `false` for an unknown `val_index`. | |
pub fn is_previous_epoch_target_attester( |
Or, arguably we should handle the Error::InvalidValidatorIndex(..)
in all those EpochProcessingSummary
functions so that the comments are accurate once again. I think this is quite safe, as those functions are only used by the analysis APIs and the validator monitor, unlike the participation cache functions themselves which are used in consensus-critical code and need to return errors for out-of-bounds validator indices.
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.
Looks good!
I've double-checked that the 4 functions modified are only used in the validator monitor and the API.
Let's merge this in v3.1.1. Will mark as blocked until v3.1.0 goes out
bors r+ |
## Issue Addressed When requesting an index which is not active during `start_epoch`, Lighthouse returns: ``` curl "http://localhost:5052/lighthouse/analysis/attestation_performance/999999999?start_epoch=100000&end_epoch=100000" ``` ```json { "code": 500, "message": "INTERNAL_SERVER_ERROR: ParticipationCache(InvalidValidatorIndex(999999999))", "stacktraces": [] } ``` This error occurs even when the index in question becomes active before `end_epoch` which is undesirable as it can prevent larger queries from completing. ## Proposed Changes In the event the index is out-of-bounds (has not yet been activated), simply return all fields as `false`: ``` -> curl "http://localhost:5052/lighthouse/analysis/attestation_performance/999999999?start_epoch=100000&end_epoch=100000" ``` ```json [ { "index": 999999999, "epochs": { "100000": { "active": false, "head": false, "target": false, "source": false } } } ] ``` By doing this, we cover the case where a validator becomes active sometime between `start_epoch` and `end_epoch`. ## Additional Info Note that this error only occurs for epochs after the Altair hard fork.
InvalidValidatorIndex
errorInvalidValidatorIndex
error
) ## Issue Addressed When requesting an index which is not active during `start_epoch`, Lighthouse returns: ``` curl "http://localhost:5052/lighthouse/analysis/attestation_performance/999999999?start_epoch=100000&end_epoch=100000" ``` ```json { "code": 500, "message": "INTERNAL_SERVER_ERROR: ParticipationCache(InvalidValidatorIndex(999999999))", "stacktraces": [] } ``` This error occurs even when the index in question becomes active before `end_epoch` which is undesirable as it can prevent larger queries from completing. ## Proposed Changes In the event the index is out-of-bounds (has not yet been activated), simply return all fields as `false`: ``` -> curl "http://localhost:5052/lighthouse/analysis/attestation_performance/999999999?start_epoch=100000&end_epoch=100000" ``` ```json [ { "index": 999999999, "epochs": { "100000": { "active": false, "head": false, "target": false, "source": false } } } ] ``` By doing this, we cover the case where a validator becomes active sometime between `start_epoch` and `end_epoch`. ## Additional Info Note that this error only occurs for epochs after the Altair hard fork.
Issue Addressed
When requesting an index which is not active during
start_epoch
, Lighthouse returns:This error occurs even when the index in question becomes active before
end_epoch
which is undesirable as it can prevent larger queries from completing.Proposed Changes
In the event the index is out-of-bounds (has not yet been activated), simply return all fields as
false
:By doing this, we cover the case where a validator becomes active sometime between
start_epoch
andend_epoch
.Additional Info
Note that this error only occurs for epochs after the Altair hard fork.