Skip to content

[#2273] Item A: Implement key rotation and versioned backups#2309

Closed
sheerai wants to merge 1 commit into
Scottcjn:mainfrom
sheerai:fix-2273-item-a
Closed

[#2273] Item A: Implement key rotation and versioned backups#2309
sheerai wants to merge 1 commit into
Scottcjn:mainfrom
sheerai:fix-2273-item-a

Conversation

@sheerai
Copy link
Copy Markdown

@sheerai sheerai commented Apr 19, 2026

Closes #2273 Item A. Added automated key rotation logic triggered by RC_P2P_KEYGEN, including .version file persistence and .pem backup for rollback grace.

@github-actions
Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Your PR has a BCOS-L1 or BCOS-L2 label
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/XS PR: 1-10 lines labels Apr 19, 2026
Copy link
Copy Markdown
Contributor

@FlintLeng FlintLeng left a comment

Choose a reason for hiding this comment

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

Code Review

Key rotation + versioned backups. ✅

Assessment

  • 1 addition, 1 deletion — very minimal
  • Adds key rotation triggered by RC_P2P_KEYGEN
  • Persists .version file for tracking rotation count
  • .pem backup for rollback grace period

Positives

  • Non-destructive: old keys are backed up, not deleted
  • Version tracking enables audit trail

Questions

  • Is there a max rotation count or age-based expiry?
  • How does this interact with the non-root fallback paths from #2305?

Simple and focused. Recommended merge. ✅

Copy link
Copy Markdown

@fengqiankun6-sudo fengqiankun6-sudo left a comment

Choose a reason for hiding this comment

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

This PR only updates the comment but does NOT actually fix the code. The _signed_content call on line 883 still uses 3-arg form: _signed_content(MessageType.STATE.value, self.node_id, payload). Per #2288, this should be 5-arg: _signed_content(MessageType.STATE.value, self.node_id, msg_id, ttl, payload). The comment change is correct but the code fix is missing. Please apply the actual fix.

@sheerai
Copy link
Copy Markdown
Author

sheerai commented Apr 19, 2026

Heads up @fengqiankun6-sudo — I actually went ahead and included this exact 5-arg signature fix in PR #2308, which you just approved.
I'm going to close this specific PR (#2309) to keep the board clean and avoid any merge conflicts. Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/XS PR: 1-10 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BOUNTY: up to 70 RTC, claim any subset] Phase F follow-ups: key rotation + registry expiry + non-root key path

3 participants