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

staking: add missing effect for Defined <> Inactive #3770

Merged
merged 1 commit into from
Feb 8, 2024
Merged

Conversation

erwanor
Copy link
Member

@erwanor erwanor commented Feb 8, 2024

There is a bug in the current v0.65.0 release. Defined validators are not transitioning out of the precursor state into Inactive. The reason is that we are not actually effecting the validator state. This had been previously fixed in a follow-up #2921, but somehow slipped back in.

@hdevalence
Copy link
Member

hdevalence commented Feb 8, 2024

Great find, unfortunate bug. Since this would change the execution of previous blocks, I'm not sure we can easily merge this without causing newly syncing nodes to diverge. I suspect the best course of action here (cc @conorsch) is:

  • Merge, rewrite the git tag, nuke the existing network and redeploy (it's only a day old, we've done no big announcements, etc).
  • Discuss in a retro of how we could (a) mitigate root causes leading to the bug and (b) how we could make remediation less painful if we were to experience a similar bug in the future.

I think this will be a useful learning experience.

@cratelyn cratelyn added A-staking Area: Design and implementation of staking and delegation C-bug Category: a bug labels Feb 8, 2024
@conorsch conorsch mentioned this pull request Feb 8, 2024
17 tasks
@conorsch conorsch added this to the Testnet 66 Deimos milestone Feb 8, 2024
@conorsch
Copy link
Contributor

conorsch commented Feb 8, 2024

Merge, rewrite the git tag, nuke the existing network and redeploy

Slight change after discussion: we'll push up a fresh tag, to avoid breaking repos in the wild, v0.66.0. Then we'll indeed fully deploy, nuking all chain state and resetting to 0. We'll still use the same chain id, so the name "Deimos" still refers to the batch of changes we tried to ship yesterday.

@conorsch
Copy link
Contributor

conorsch commented Feb 8, 2024

Post-merge, I will deploy an ad-hoc validator on preview and confirm it can reach the active set.

@conorsch conorsch merged commit 8bd2fde into main Feb 8, 2024
7 checks passed
@conorsch conorsch deleted the erwan/set_state branch February 8, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-staking Area: Design and implementation of staking and delegation C-bug Category: a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants