Skip to content

test: Add registry expiry validation tests (#2273 Item B)#2633

Closed
BossChaos wants to merge 2 commits into
Scottcjn:mainfrom
BossChaos:feature/registry-expiry-item-b-2273
Closed

test: Add registry expiry validation tests (#2273 Item B)#2633
BossChaos wants to merge 2 commits into
Scottcjn:mainfrom
BossChaos:feature/registry-expiry-item-b-2273

Conversation

@BossChaos
Copy link
Copy Markdown
Contributor

Summary

Implements Item B from #2273 — Registry expiry / not_before / not_after validation with comprehensive test coverage.

Implementation Status

Core logic already implemented in p2p_identity.py:

  • PeerEntry dataclass has not_before / not_after fields (ISO-8601)
  • get_pubkey() validates time window with ±5 min clock skew tolerance
  • get_entry_with_version() combines version + expiry checks
  • Expired/not-yet-valid entries logged and rejected

This PR

Adds 7 comprehensive regression tests validating all acceptance criteria:

  1. ✅ PeerEntry accepts not_before/not_after fields
  2. ✅ Valid time window entries accepted
  3. ✅ Expired entries (past not_after) rejected
  4. ✅ Not-yet-valid entries (before not_before) rejected
  5. ✅ Clock skew tolerance (±300s) applied correctly
  6. ✅ Version + expiry combined checks work together
  7. ✅ Null time fields mean no restriction

Test Results

python3 test_p2p_registry_expiry.py
# Results: 7 passed, 0 failed

Related

…tem A)

- Add key_version field to LocalKeypair, persisted in .version file
- Add RC_P2P_KEYGEN=1 env var to force key rotation with version increment
- Add key_version to PeerRegistry entries for version-aware verification
- Add get_entry_with_version() to validate signature version matches registry
- Archive old keys as .v{N}.pem on rotation
- Add comprehensive regression tests (5 test cases)

Acceptance criteria met:
✅ Key version persisted alongside PEM
✅ Version checked during signature verification
✅ Old versions rejected after rotation
✅ New versions accepted
7 comprehensive test cases covering:
- PeerEntry with not_before/not_after fields
- Valid time window acceptance
- Expired entry rejection (not_after)
- Not-yet-valid entry rejection (not_before)
- Clock skew tolerance (±5 minutes)
- Combined version + expiry checks
- Null time fields (no restriction)

All tests passing.
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes labels Apr 22, 2026
@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!

@Scottcjn
Copy link
Copy Markdown
Owner

Superseded — bounty already paid

Item(s) from issue #2273 were fully implemented in PR #2296 by @MichaelSovereign (merged 2026-04-19, 70 RTC paid).

Your implementation here is technically correct — the race happened on Apr 19 and your PR arrived Apr 21-22. Closing this as superseded, not as spam or low quality.

Why closing rather than partially merging:

  • The shipped code in node/p2p_identity.py already has key_version / not_before / not_after / path fallback
  • Layering another implementation on top would conflict with the merged structure
  • Tests you've written are good — if there's a path-fallback test gap in mainline, a narrow follow-up PR adding just those tests (≤50 LOC, no logic changes) would be welcome

Keep watching issues. Claim early, deliver fast, you'll land one.

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/XL PR: 500+ lines tests Test suite changes

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

2 participants