Skip to content

fix: [PM-19967] Bound GRANDPA and BEEFY finality subscription fan-out#1075

Open
m2ux wants to merge 8 commits intomainfrom
fix/PM-19967-unbounded-finality-subscription-fanout
Open

fix: [PM-19967] Bound GRANDPA and BEEFY finality subscription fan-out#1075
m2ux wants to merge 8 commits intomainfrom
fix/PM-19967-unbounded-finality-subscription-fanout

Conversation

@m2ux
Copy link
Copy Markdown
Contributor

@m2ux m2ux commented Mar 25, 2026

Summary

Enforce a global limit on concurrent GRANDPA and BEEFY finality RPC subscriptions to prevent resource exhaustion from unbounded fan-out of consensus notifications. Addresses Medium-severity audit finding Issue W from the Least Authority security audit.

🎫 Ticket 📐 Engineering 🧪 Test Plan


Motivation

The node's GRANDPA and BEEFY RPC subscription handlers fan out consensus finality notifications via unbounded channels (tracing_unbounded). An attacker can open many WebSocket connections, subscribe to finality streams, and let buffers grow without limit — degrading node responsiveness or triggering a crash. This was identified as Issue W (Medium severity) in the Least Authority initial audit report (October 2025), affecting the node at git revision d204679f.

While Substrate's jsonrpsee server exposes --rpc-max-connections and --rpc-max-subscriptions-per-connection flags, these are transport-level limits that do not bound the underlying notification channels from the consensus layer.


Changes

  • subscription_bounds module (new) — SubscriptionTracker using atomic CAS for concurrent slot management, SubscriptionGuard (RAII) for automatic cleanup on client disconnect, SubscriptionMetrics for Prometheus monitoring (midnight_rpc_finality_subscriptions_active gauge, midnight_rpc_finality_subscriptions_rejected_total counter)
  • rpc.rs — Replace upstream GRANDPA and BEEFY subscription handlers with bounded variants using a merge-remove-replace pattern; generic register_bounded_finality_subscription<T, TK> helper eliminates duplication between the two protocols
  • cli.rs — New --rpc-max-finality-subscriptions flag (default 512) for operator configuration
  • service.rs — Create tracker with optional Prometheus metrics registration, pass through to RPC builder
  • Tests — 5 unit tests: happy path, boundary enforcement, RAII guard drop, zero-limit edge case, and concurrent access (100-thread barrier synchronization). All 55 existing unit tests + 3 integration tests continue to pass.

📌 Submission Checklist

  • Changes are backward-compatible (or flagged if breaking)
  • Pull request description explains why the change is needed
  • Self-reviewed the diff
  • I have included a change file, or skipped for this reason: included
  • If the changes introduce a new feature, I have bumped the node minor version
  • Update documentation (if relevant)
  • No new todos introduced

🔱 Fork Strategy

  • Node Runtime Update
  • Node Client Update
  • Other
  • N/A

🗹 TODO before merging

  • Ready for review

m2ux added 7 commits March 25, 2026 11:27
Introduces SubscriptionTracker with RAII-based SubscriptionGuard for
enforcing configurable global limits on concurrent GRANDPA/BEEFY
subscriptions. Includes SubscriptionMetrics for Prometheus monitoring
(active gauge, rejected counter). Unit tests cover limit enforcement,
guard lifecycle, and concurrent access.

JIRA: https://shielded.atlassian.net/browse/PM-19967
Replace upstream GRANDPA and BEEFY subscription handlers with bounded
versions that check a shared SubscriptionTracker before accepting new
subscriptions. Non-subscription methods (roundState, proveFinality,
getFinalizedHead) remain delegated to the upstream handlers.

Each subscription acquires a SubscriptionGuard (RAII) that decrements
the active count when the subscriber disconnects. The limit is
configurable via --rpc-max-finality-subscriptions (default: 512).

Includes Prometheus metrics registration for active and rejected
subscription counts.

JIRA: https://shielded.atlassian.net/browse/PM-19967
Use a barrier to synchronize thread acquisition so guards remain held
when asserting active_count, preventing premature guard drops.
…into fix/PM-19967-unbounded-finality-subscription-fanout
- Remove unused SubscriptionBoundsConfig struct (dead code)
- Extract duplicated GRANDPA/BEEFY subscription registration into
  generic register_bounded_finality_subscription helper
- Add warn! log on subscription rejection for operator visibility
- Await pending.reject() future to resolve unused-must-use warning
@m2ux m2ux marked this pull request as ready for review March 25, 2026 17:38
@m2ux m2ux requested a review from a team as a code owner March 25, 2026 17:38
@m2ux m2ux self-assigned this Mar 26, 2026
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.

1 participant