Skip to content

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

Merged
m2ux merged 8 commits into
mainfrom
fix/PM-19967-unbounded-finality-subscription-fanout
Apr 15, 2026
Merged

fix: [PM-19967] Bound GRANDPA and BEEFY finality subscription fan-out#1075
m2ux merged 8 commits into
mainfrom
fix/PM-19967-unbounded-finality-subscription-fanout

Conversation

@m2ux

@m2ux m2ux commented Mar 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 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
@m2ux m2ux force-pushed the fix/PM-19967-unbounded-finality-subscription-fanout branch from 24e84e3 to 9bee69c Compare April 13, 2026 09:28
@m2ux m2ux enabled auto-merge April 13, 2026 09:31
@m2ux m2ux mentioned this pull request Apr 13, 2026
Closed
m2ux and others added 7 commits April 13, 2026 15:46
@m2ux
chore: add change file for PM-19967 finality subscription bounds
0d59c61 
Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux
chore: update change file with PR link
84a8e7c 
Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux
feat: add subscription_bounds module for finality subscription limits
219a31a 
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
Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux
feat: enforce global finality subscription limit for GRANDPA and BEEFY
4b0fda6 
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
Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux
fix: correct concurrent access test for subscription tracker
2aa58a6 
Use a barrier to synchronize thread acquisition so guards remain held
when asserting active_count, preventing premature guard drops.

Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux
refactor: address code review findings for subscription bounds
bb15f11 
- 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

Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux
chore: add GitHub issue link to change file for PM-19967
58701e0 
Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux m2ux force-pushed the fix/PM-19967-unbounded-finality-subscription-fanout branch from 8f35996 to 58701e0 Compare April 13, 2026 14:46
@m2ux m2ux added this pull request to the merge queue Apr 15, 2026
Merged via the queue into main with commit bd2a8af Apr 15, 2026
31 of 33 checks passed
@m2ux m2ux deleted the fix/PM-19967-unbounded-finality-subscription-fanout branch April 15, 2026 17:53
m2ux added a commit that referenced this pull request Apr 23, 2026
@m2ux
fix: [PM-19967] Bound GRANDPA and BEEFY finality subscription fan-out (…
d6a1333 
…#1075)

* chore: add change file for PM-19967 finality subscription bounds


* chore: update change file with PR link


* feat: add subscription_bounds module for finality subscription limits

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

* feat: enforce global finality subscription limit for GRANDPA and BEEFY

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

* fix: correct concurrent access test for subscription tracker

Use a barrier to synchronize thread acquisition so guards remain held
when asserting active_count, preventing premature guard drops.


* refactor: address code review findings for subscription bounds

- 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


* chore: add GitHub issue link to change file for PM-19967


---------


Signed-off-by: Mike Clay <mike.clay@shielded.io>
m2ux added a commit that referenced this pull request Apr 23, 2026
@m2ux
fix: [PM-19967] Bound GRANDPA and BEEFY finality subscription fan-out (…
95bdae7 
…#1075)

* chore: add change file for PM-19967 finality subscription bounds


* chore: update change file with PR link


* feat: add subscription_bounds module for finality subscription limits

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

* feat: enforce global finality subscription limit for GRANDPA and BEEFY

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

* fix: correct concurrent access test for subscription tracker

Use a barrier to synchronize thread acquisition so guards remain held
when asserting active_count, preventing premature guard drops.


* refactor: address code review findings for subscription bounds

- 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


* chore: add GitHub issue link to change file for PM-19967


---------


Signed-off-by: Mike Clay <mike.clay@shielded.io>
@m2ux m2ux linked an issue Apr 29, 2026 that may be closed by this pull request
[Node] Issue W: Unbounded Finality Subscription Fan-out #1316
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinfrevert justinfrevert justinfrevert approved these changes

Assignees

@m2ux m2ux

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Node] Issue W: Unbounded Finality Subscription Fan-out

2 participants

@m2ux @justinfrevert