Skip to content
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

Only active validator signs stats #1508

Merged
merged 4 commits into from
Apr 14, 2021

Conversation

gastonponti
Copy link
Contributor

@gastonponti gastonponti commented Apr 13, 2021

Description

A proxy with two validators (one active) that send a stat to be signed by the validator, was receiving 2 signed messages for the same (one per validator)

Only the active validator will sign those stats.

Other changes

The validators MUST have the celostats flag too (as we have it in the docs)

Tested

Manually

Backwards compatibility

Breaking change in that proxied validators need to specify the celostats flag in order to report to celostats. However, this new behavior agrees with our documentation, so this is really more of a bugfix.

@gastonponti gastonponti requested a review from a team as a code owner April 13, 2021 22:05
@gastonponti gastonponti requested review from kevjue and trianglesphere and removed request for a team April 13, 2021 22:05
ethstats/ethstats.go Outdated Show resolved Hide resolved
Co-authored-by: Or Neeman <or@clabs.co>
Copy link
Contributor

@oneeman oneeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I made a suggested change in one of the error messages, and edited the "backwards compatibility" section to call out the breaking change also described in "Other changes".

Copy link
Contributor

@nategraf nategraf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@gastonponti gastonponti merged commit 9e6f275 into master Apr 14, 2021
@gastonponti gastonponti deleted the gastonponti/celostats-multiple-validators branch April 14, 2021 15:21
oneeman pushed a commit that referenced this pull request Apr 14, 2021
Description
A proxy with two validators (one active) that send a stat to be signed by the validator, was receiving 2 signed messages for the same (one per validator)

Only the active validator will sign those stats.

Other changes
The validators MUST have the celostats flag too (as we have it in the docs)

Tested
Manually

Backwards compatibility
Breaking change in that proxied validators need to specify the celostats flag in order to report to celostats. However, this new behavior agrees with our documentation, so this is really more of a bugfix.
oneeman pushed a commit that referenced this pull request Apr 14, 2021
A proxy with two validators (one active) that send a stat to be signed by the validator, was receiving 2 signed messages for the same (one per validator)

Only the active validator will sign those stats.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants