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

fix: allow unbonded validator to submit heartbeats #1256

Merged
merged 2 commits into from
Jan 25, 2022
Merged

Conversation

milapsheth
Copy link
Member

@milapsheth milapsheth commented Jan 25, 2022

Description

Due to a change in #1198 for missed blocks, we started returning an error for any validator whose signing info wasn't found. This includes any validator who's newly created but hasn't joined the active set yet. Also, any validator who gets unbonded will have missed blocks. Since we want to allow validators who are unbonding/unbonded to still sign for existing key IDs, we shouldn't exclude them from based on this illegibility.

This PR changes the missed blocks check to only be applicable for bonded validators.

Alternative approaches:

  • Filter sign illegibility for missed blocks, like we do for proxy deregistered. But this filter will also apply to bonded validators who are missing blocks, so it would require some refactoring.
  • Include all validators from the snapshot in sign, due to robustness. This will be an issue for non robust threshold schemes later. And if they don't respond, we might suspend validators unnecessarily.

Todos

  • Unit tests
  • Manual tests
  • Documentation
  • Connect epics/issues
  • Tag type of change

Steps to Test

Expected Behaviour

Other Notes

Copy link
Contributor

@jcs47 jcs47 left a comment

Choose a reason for hiding this comment

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

LGTM

@milapsheth milapsheth enabled auto-merge (squash) January 25, 2022 22:07
@milapsheth milapsheth merged commit e72fa94 into main Jan 25, 2022
@milapsheth milapsheth deleted the missed-blocks branch January 25, 2022 22:11
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