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

Remove signers older than the current reward cycle if they have no more blocks to process #5562

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

jferrant
Copy link
Collaborator

@jferrant jferrant commented Dec 11, 2024

We used to keep reward cycle signers for an extra reward cycle in prep for potentially the fallback scenario where signers would continue signing blocks/sbtc transactions if the incoming signers failed to initialize. However, this is really confusing our partners who don't understand why their logs report stale signers as operating. Also this old logic was flawed anyway as we only ever kept 2 signers around anyway (the hashmap of signers is keyed by the reward_cycle % 2). The cleanup function as it was essentially was a no-op.

This code change makes the function delete anything that has no unprocessed blocks in its database and that is for a reward cycle older than the current reward cycle. We can always revert or add additional logic about when to delete them when we start giving potential responsibilities to "stale" signers.

Closes #5507

@jferrant jferrant requested a review from a team as a code owner December 11, 2024 15:49
jcnelson
jcnelson previously approved these changes Dec 11, 2024
@jcnelson
Copy link
Member

It seems this broke a bunch of things.

@jcnelson jcnelson self-requested a review December 11, 2024 19:53
@jferrant
Copy link
Collaborator Author

jferrant commented Dec 11, 2024

It seems this broke a bunch of things.

No kidding....Taking a look! EDIT: oh....I had the if statement backwards...don't judge me...

Signed-off-by: Jacinta Ferrant <[email protected]>
@jferrant jferrant requested a review from a team as a code owner December 12, 2024 14:26
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

LGTM!

@jferrant jferrant added this pull request to the merge queue Dec 13, 2024
Merged via the queue into develop with commit b3b7117 Dec 13, 2024
11 checks passed
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Dec 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants