Refine implementation plan based on review feedback

- Add manual review requirements for research and dashboard planning phases
- Clarify NIP-86 server-side implementation can proceed if rust-nostr supports it
- Add dependency on deletion request issue for repository cleanup phase
- Remove defensive measures phase (not in original architecture)
- Consolidate abuse metrics display into dashboard phase using existing ConnectionTracker
- Update phase dependencies, effort estimates, and success criteria
- Reduce total timeline from 31-39 days to 26-37 days
This commit is contained in:
DanConwayDev
2026-01-14 16:36:03 +00:00
parent 45013a8891
commit 801d4eaf0a
+65 -87
View File
@@ -704,6 +704,8 @@ This section breaks down the implementation into phases with clear dependencies
**Goal:** Understand existing implementations and verify technical feasibility of key architectural decisions.
**⚠️ Manual Review Required:** This phase produces research findings that inform architectural decisions in later phases. Results must be reviewed and approved before proceeding to implementation phases.
**Parallelizable Tasks:**
**Track A: NIP-86 Research**
@@ -722,6 +724,8 @@ This section breaks down the implementation into phases with clear dependencies
- Evaluate NIP-44 encryption fallback if direct response not supported
- Deliverable: Technical feasibility report with implementation approach
**⚠️ Manual Review Required:** Track B findings determine the implementation approach for Phase 2 Track B. Must be reviewed and approved before starting Phase 2 Track B work.
**Effort:** 2-3 days (parallel tracks)
**Dependencies:** None
@@ -848,6 +852,8 @@ This section breaks down the implementation into phases with clear dependencies
**Goal:** Implement authenticated management API for administrative operations.
**Note:** This phase can run unattended IF the rust-nostr library (nostr-sdk) supports NIP-86. If manual implementation is required, this becomes a manual review phase.
**Tasks:**
- Implement NIP-86 relay management protocol (based on Phase 0 Track A research)
- Add `NGIT_ADMIN_PUBKEYS` environment variable (comma-separated hex pubkeys)
@@ -864,7 +870,7 @@ This section breaks down the implementation into phases with clear dependencies
- Implement `NGIT_DECLARATIVE_ONLY` flag behavior
- Testing: Test each operation, verify auth, test declarative vs dynamic modes
**Effort:** 4-5 days
**Effort:** 4-5 days (IF rust-nostr supports NIP-86), 6-8 days (if manual implementation)
**Dependencies:**
- Phase 0 Track A (NIP-86 implementation approach - must be complete)
@@ -884,6 +890,13 @@ This section breaks down the implementation into phases with clear dependencies
**Goal:** Provide human-friendly interface for administrators.
**⚠️ Manual Review Required:** Detailed implementation plan for the dashboard must be reviewed and approved before starting implementation. Plan should include:
- Selected framework/technology (based on Phase 0 research)
- UI/UX design approach
- Feature prioritization
- Integration approach with NIP-86 API
- Testing strategy
**Tasks:**
- Research and select dashboard framework (based on Phase 0 Track A NIP-86 UI findings)
- Option 1: Reuse/adapt existing NIP-86 management UI
@@ -896,6 +909,7 @@ This section breaks down the implementation into phases with clear dependencies
- Repository management (search, view size, blacklist, set quota)
- Configuration viewer (read-only display of current config)
- API key management (generate Prometheus keys, list, revoke)
- **Abuse metrics display** (show existing ConnectionTracker data: connection patterns, invalid signatures, filter violations)
- Implement NIP-86 client (connect to management API from Phase 4)
- Extend NIP-86 API with API key operations:
- Generate new Prometheus API key (return unhashed, store hashed)
@@ -907,6 +921,7 @@ This section breaks down the implementation into phases with clear dependencies
**Effort:** 6-8 days
**Dependencies:**
- Phase 0 Track A (NIP-86 UI research - must be complete and approved)
- Phase 4 (requires NIP-86 API)
- Phase 3 (requires Prometheus auth for API key management)
- Phase 1 (requires metrics to display)
@@ -914,6 +929,7 @@ This section breaks down the implementation into phases with clear dependencies
**Related Issues:**
- Addresses 7d0b (Management Dashboard) - **PRIMARY ISSUE**
- Completes 2cdc (NIP-86 API now has UI)
- Partially addresses 1f4f (displays abuse metrics from ConnectionTracker)
**Within-Phase Parallelization:** Limited. Dashboard UI and API key operations could be developed in parallel initially, but must integrate before completion.
@@ -957,11 +973,13 @@ This section breaks down the implementation into phases with clear dependencies
**Dependencies:**
- Phase 4 (requires quota storage from NIP-86 API)
- Phase 5 (can run unattended once dashboard is approved and complete)
- Phase 1 Track B (requires storage calculation)
- Issue b905 (Deletion Request Support - NIP-09 implementation)
**Related Issues:**
- Addresses 8430 (Storage Limits and Quota Management) - **PRIMARY ISSUE**
- Partially addresses b905 (Deletion Request Support) if we add NIP-09 support
- Depends on b905 (Deletion Request Support) for NIP-09 event deletion
**Within-Phase Parallelization:** None. Repository deletion and quota enforcement are tightly coupled to the same code paths (git handler, event handler). Must be implemented sequentially.
@@ -969,59 +987,15 @@ This section breaks down the implementation into phases with clear dependencies
---
### Phase 7: Defensive Measures & Abuse Detection
**Goal:** Automatic protection from abuse and resource exhaustion.
**Tasks:**
**Track A: Rate Limiting & Connection Limits**
- Implement global rate limits (from configuration)
- Add `NGIT_MAX_CONNECTIONS` environment variable
- Add `NGIT_RATE_LIMIT_EVENTS_PER_MINUTE` environment variable
- Add `NGIT_RATE_LIMIT_GIT_OPS_PER_MINUTE` environment variable
- Enforce limits in WebSocket handler and git handler
- Return clear error messages when limits exceeded
- Add metrics for rate limit violations
- Testing: Test each limit type, verify enforcement, test error messages
**Track B: Abuse Pattern Detection**
- Implement abuse detection (based on existing ConnectionTracker)
- Detect invalid signatures (repeated failures from same pubkey)
- Detect filter violations (requests violating relay policy)
- Detect DoS patterns (excessive connections, requests)
- Implement automatic temporary bans
- Add ban duration configuration
- Store temporary bans in memory (cleared on restart)
- Escalate to permanent ban after N temporary bans
- Integrate with blacklist system from Phase 4
- Add metrics for abuse detection events
- Testing: Test each detection pattern, test ban escalation, test integration
**Effort:** 4-5 days
**Dependencies:**
- Phase 4 (requires blacklist system from NIP-86 API)
- Existing ConnectionTracker implementation
**Related Issues:**
- Addresses d6ee (Defensive Relay Features) - Phase 1 only (config-based)
- Addresses 1f4f (Poor Naughty List Identification) - **PRIMARY ISSUE**
**Within-Phase Parallelization:** Rate limiting and abuse detection are independent mechanisms that can be developed in parallel by different developers or agents.
**Deliverable:** Commit rate limiting and abuse detection with tests. All existing tests must pass.
---
### Summary: Phase Dependencies & Parallelization
```
Phase 0: Research (2-3 days)
Phase 0: Research (2-3 days) ⚠️ MANUAL REVIEW REQUIRED
├─ Track A: NIP-86 Research ─────────────┐
└─ Track B: nostr-relay-builder Research ─┴─► (within-phase parallel)
Dependencies: None
Deliverable: Research documentation
Manual Review: Track B before Phase 2 Track B
Phase 1: Foundation (4-5 days)
├─ Track A: Stats Cache ─────────────────┐
@@ -1032,7 +1006,7 @@ Phase 1: Foundation (4-5 days)
Phase 2: Public & User Info (3-4 days)
├─ Track A: NIP-11 Extension ────────────┐
└─ Track B: User Storage Events ─────────┴─► (within-phase parallel)
Dependencies: Phase 1 complete, Phase 0 Track B
Dependencies: Phase 1 complete, Phase 0 Track B (approved)
Deliverable: Working code + tests
Phase 3: Prometheus Auth (2-3 days)
@@ -1040,40 +1014,40 @@ Phase 3: Prometheus Auth (2-3 days)
Dependencies: Phase 1 complete
Deliverable: Working code + tests
Phase 4: NIP-86 Management API (4-5 days)
└─ Single track
Phase 4: NIP-86 Management API (4-8 days)
└─ Single track (can run unattended IF rust-nostr supports NIP-86)
Dependencies: Phase 0 Track A, Phase 1 Track B
Deliverable: Working code + tests
Note: 4-5 days if supported, 6-8 days if manual implementation
Phase 5: Management Dashboard (6-8 days)
Phase 5: Management Dashboard (6-8 days) ⚠️ MANUAL REVIEW REQUIRED
└─ Single track (limited internal parallelization)
Dependencies: Phase 3, Phase 4, Phase 1
Dependencies: Phase 0 Track A (approved), Phase 3, Phase 4, Phase 1
Deliverable: Working code + tests
Manual Review: Detailed plan before implementation
Phase 6: Quota Enforcement (5-6 days)
└─ Sequential tasks within phase
Dependencies: Phase 4, Phase 1 Track B
Deliverable: Working code + tests
Phase 7: Defensive Measures (4-5 days)
├─ Track A: Rate Limiting ───────────────┐
└─ Track B: Abuse Detection ─────────────┴─► (within-phase parallel)
Dependencies: Phase 4
└─ Sequential tasks within phase (can run unattended once Phase 5 approved)
Dependencies: Phase 4, Phase 5 (approved), Phase 1 Track B, Issue b905
Deliverable: Working code + tests
```
**Total Sequential Time:** ~31-39 days (6-8 weeks)
**Total Sequential Time:** ~26-34 days (5-7 weeks)
**Parallelization Opportunities:**
- **Within-phase:** Phases 0, 1, 2, and 7 have independent tracks that can run simultaneously
- **Within-phase:** Phases 0, 1, and 2 have independent tracks that can run simultaneously
- **Cross-phase:** Limited. Most phases have strict dependencies on prior phases
- **Realistic speedup:** With 2 developers working parallel tracks: ~25-30 days (5-6 weeks)
- **Realistic speedup:** With 2 developers working parallel tracks: ~22-28 days (4-6 weeks)
**Recommended Approach:**
- Single developer: Follow phases sequentially (0→1→2→3→4→5→6→7), executing parallel tracks within each phase
- Single developer: Follow phases sequentially (0→1→2→3→4→5→6), executing parallel tracks within each phase
- Team of 2: One developer per track in phases with parallelizable tracks; alternate on single-track phases
- Larger teams: Not recommended - phases have strict sequential dependencies
**Manual Review Points:**
1. Phase 0 Track B results (before Phase 2 Track B)
2. Phase 5 detailed plan (before Phase 5 implementation)
### Issue Mapping
**Issues Addressed by Implementation:**
@@ -1084,35 +1058,40 @@ Phase 7: Defensive Measures (4-5 days)
| 7d0b (Management Dashboard) | Phase 2 Track A, Phase 5 | Primary issue for Phase 5 |
| 2cdc (NIP-86 Relay Management API) | Phase 0 Track A, Phase 4, Phase 5 | Primary issue for Phase 4 |
| 8430 (Storage Limits & Quota Management) | Phase 4, Phase 6 | Primary issue for Phase 6 |
| d6ee (Defensive Relay Features) | Phase 7 Track A | Phase 1 only (config-based) |
| 1f4f (Poor Naughty List Identification) | Phase 7 Track B | Primary issue |
| b905 (Deletion Request Support) | Phase 6 | Partially (if NIP-09 added) |
| 1f4f (Poor Naughty List Identification) | Phase 5 | Partially (display existing ConnectionTracker metrics in dashboard) |
| b905 (Deletion Request Support) | Phase 6 | Dependency (NIP-09 must be implemented before Phase 6) |
**Issues NOT Addressed:**
| Issue | Reason |
|-------|--------|
| d6ee (Defensive Relay Features) | Removed from this plan (was Phase 7 Track A) - should be separate issue/plan |
**Issues Superseded:**
None. All existing issues remain relevant and are incorporated into this plan.
None. Most existing issues remain relevant and are incorporated into this plan.
### Effort Estimates
**By Phase:**
- Phase 0: 2-3 days (research)
- Phase 0: 2-3 days (research, requires manual review)
- Phase 1: 4-5 days (foundation)
- Phase 2: 3-4 days (public/user info)
- Phase 3: 2-3 days (Prometheus auth)
- Phase 4: 4-5 days (NIP-86 API)
- Phase 5: 6-8 days (dashboard)
- Phase 6: 5-6 days (quota enforcement)
- Phase 7: 4-5 days (defensive measures)
- Phase 4: 4-8 days (NIP-86 API - depends on rust-nostr support)
- Phase 5: 6-8 days (dashboard, requires manual review of plan)
- Phase 6: 5-6 days (quota enforcement, can run unattended once Phase 5 approved)
**Total:** 31-39 days (6-8 weeks) sequential, 25-30 days (5-6 weeks) with within-phase parallelization
**Total:** 26-37 days (5-7 weeks) sequential, 22-30 days (4-6 weeks) with within-phase parallelization
**By Issue:**
- 76fe: 0 days (verification only)
- 7d0b: 9-12 days (Phase 2 Track A + Phase 5)
- 2cdc: 8-10 days (Phase 0 Track A + Phase 4 + Phase 5 API key mgmt)
- 8430: 9-11 days (Phase 4 quota storage + Phase 6)
- d6ee: 2-3 days (Phase 7 Track A)
- 1f4f: 2-3 days (Phase 7 Track B)
- 2cdc: 8-13 days (Phase 0 Track A + Phase 4 + Phase 5 API key mgmt)
- 8430: 9-14 days (Phase 4 quota storage + Phase 6)
- 1f4f: Included in Phase 5 (dashboard displays existing ConnectionTracker data)
- b905: Dependency (must be complete before Phase 6)
- d6ee: NOT ADDRESSED (removed from this plan)
### Risk Mitigation
@@ -1142,6 +1121,7 @@ None. All existing issues remain relevant and are incorporated into this plan.
- [ ] NIP-86 research document published with implementation recommendations
- [ ] nostr-relay-builder feasibility report with chosen approach for user storage events
- [ ] Research committed to `docs/research/` directory
- [ ] **Manual review completed and Phase 0 Track B approach approved**
**Phase 1 Complete:**
- [ ] Stats cache implemented and tested (60-second throttle)
@@ -1169,30 +1149,28 @@ None. All existing issues remain relevant and are incorporated into this plan.
- [ ] All tests pass, changes committed
**Phase 5 Complete:**
- [ ] **Detailed dashboard plan reviewed and approved before implementation**
- [ ] Management dashboard accessible and functional
- [ ] Dashboard displays all metrics with visualizations
- [ ] Dashboard shows existing abuse metrics from ConnectionTracker
- [ ] Administrators can perform all management operations via UI
- [ ] API keys can be generated and managed via UI
- [ ] All tests pass, changes committed
**Phase 6 Complete:**
- [ ] Issue b905 (NIP-09 Deletion Request Support) implemented and merged
- [ ] Repositories can be deleted via management API/UI
- [ ] Repository deletion integrates with NIP-09 event deletion
- [ ] Storage quotas enforced on git push and repository creation
- [ ] Clear error messages guide users when quotas exceeded
- [ ] Graceful degradation when approaching relay-wide limits
- [ ] All tests pass, changes committed
**Phase 7 Complete:**
- [ ] Rate limits enforced (connections, events, git operations)
- [ ] Abuse patterns detected and logged
- [ ] Automatic temporary bans for repeated violations
- [ ] Integration with blacklist system for permanent bans
- [ ] All tests pass, changes committed
**Overall Success:**
- [ ] Administrator can monitor relay health without SSH access
- [ ] Administrator can view abuse metrics (from ConnectionTracker) in dashboard
- [ ] Administrator can take corrective action through dashboard
- [ ] Relay automatically protects itself from common abuse scenarios
- [ ] Storage quotas protect relay from resource exhaustion
- [ ] All workflows from strategy document are supported
- [ ] All three audiences (public, users, admins) have appropriate access to information