Skip to content

feat: key rotation #5777

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

Open
wants to merge 47 commits into
base: develop
Choose a base branch
from
Open

feat: key rotation #5777

wants to merge 47 commits into from

Conversation

jstuczyn
Copy link
Contributor

@jstuczyn jstuczyn commented May 16, 2025

that one was definitely another ecach-size adventure. you can find detailed description on our confluence page

This change is Reviewable

@jstuczyn jstuczyn added this to the Cheddar milestone May 16, 2025
Copy link

vercel bot commented May 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nym-explorer-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 29, 2025 9:33am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs-nextra ⬜️ Ignored (Inspect) Visit Preview May 29, 2025 9:33am
nym-next-explorer ⬜️ Ignored (Inspect) Visit Preview May 29, 2025 9:33am

Copy link
Contributor

@durch durch left a comment

Choose a reason for hiding this comment

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

This is all very well thought out and executed. I've got to conceptual comments, or weak points, but nothing that we can address, more something to be vigilant about:

  1. The config upgrade process, particularly upgrade_sphinx_key, involves network calls and multiple file operations. A failure mid-process could leave the node in a partially upgraded or broken state.
  2. The KeyRotationController's error handling for failed key/filter operations (that don't poison a mutex) primarily relies on logging. Depending on the failure, this might lead to the node operating with inconsistent keys/filters until the next successful cycle.

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.

2 participants