Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); p2p: libp2p resource manager with configured limits by pinebit · Pull Request #4639 · ObolNetwork/charon · GitHub
Skip to content

p2p: libp2p resource manager with configured limits - #4639

Open
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager
Open

p2p: libp2p resource manager with configured limits#4639
pinebit wants to merge 3 commits into
mainfrom
pinebit/libp2p-resource-manager

Conversation

@pinebit

@pinebitpinebit commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Replace libp2p's NullResourceManager, which never rejects anything and leaves nodes open to stream, connection and memory exhaustion, with configured resource managers.

Validator nodes use autoscaled system limits with fixed per-peer limits: defaults for relays and unknown peers, elevated but bounded limits for authenticated cluster peers, raised protocol-scope defaults for charon protocols, and per-IP connection limits that allow cluster peers to share an IP (NAT or local clusters). This path is gated behind the new alpha feature flag libp2p_resource_manager and falls back to the null manager when disabled.

Relays derive limits from their existing connection config (p2p-max-connections, p2p-max-reservations) unconditionally: system connection, FD and stream limits track the configured capacity, transient limits match system limits to survive reconnect storms, and the circuit relay hop/stop protocol and relay service scopes are sized to full circuit load instead of go-libp2p's memory-scaled defaults, which would otherwise cap circuits far below the configured capacity.

Both managers replace go-libp2p's default per-IP connection rate limiter (burst of 16 connections per IP regardless of configured limits) with one sized to the per-IP connection limits, so reconnect bursts from peers behind a shared NAT IP are not throttled.

category: feature
ticket: none

Replace NullResourceManager (which never rejects anything, leaving nodes
open to stream, connection and memory exhaustion) with configured
resource managers. Validator nodes get autoscaled system limits with
fixed per-peer limits, elevated for authenticated cluster peers, gated
behind the alpha libp2p_resource_manager feature flag. Relays derive
limits from their connection config unconditionally, including circuit
relay protocol and service scopes and a per-IP connection rate limiter
sized to the per-IP connection limits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pinebit
pinebit requested a lite review from CopilotAugust 10, 2026 19:02
@pinebitpinebit changed the title p2p: replace null libp2p resource manager with configured limitsp2p: libp2p resource manager with configured limitsAug 10, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces configured libp2p resource managers (instead of network.NullResourceManager) to enforce connection/stream/memory limits and reduce DoS/resource-exhaustion risk. It adds a feature flag for validator nodes while applying capacity-derived limits for relay nodes.

Changes:

  • Add validator and relay resource manager implementations with per-peer/per-protocol/per-subnet limits and a custom per-IP connection rate limiter.
  • Gate validator resource manager behind the new alpha feature flag libp2p_resource_manager, falling back to the null manager when disabled.
  • Update relay startup to use the configured resource manager and add internal tests validating key limit behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
p2p/resourcemanager.goImplements configured libp2p resource managers and limit/rate-limiter helpers.
p2p/resourcemanager_internal_test.goAdds tests asserting per-peer stream limits, relay conn limits, and burst behavior.
cmd/relay/p2p.goSwitches relay node startup from null to configured resource manager.
app/featureset/featureset.goAdds libp2p_resource_manager feature flag definition and registration.
app/app.goGates validator node resource manager usage behind the new feature flag.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadapp/app.go Outdated
Comment threadcmd/relay/p2p.go
p2p.NewNode can fail before libp2p takes ownership of the resource
manager, leaking its background goroutines. Close it on the error path;
Close is idempotent so paths where libp2p already closed it are safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set explicit system and transient stream and connection limits instead of
inheriting go-libp2p's autoscaled defaults. Those derive from an eighth of
host memory and fall below the fixed per-peer limits on typical validator
hardware: 3072 system inbound streams and 63 transient inbound connections
on a 16GB host, against a 4096 stream cluster peer allowance and a 64
connection per-IP burst. The per-peer limits were therefore unreachable and
a single peer could exhaust the system stream budget. Memory and FD limits
stay host derived, since those track resources the host actually has.
This fixes TestNewResourceManagerPeerStreamLimits and
TestConnRateLimiterAllowsPerIPBurst, which passed only on hosts with enough
memory for the autoscaled ceilings to clear the fixed limits. The limit
config moves into clusterLimitConfig, mirroring relayLimitConfig, and
TestClusterLimitConfig asserts the limits no longer track host memory.
streamLimits takes separate inbound and outbound values so the protocol
scopes track each direction's system and peer limit rather than deriving
both from the inbound one. Built limits are unchanged by that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecovBot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.06250% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.31%. Comparing base (aea5631) to head (59f9b82).

Files with missing linesPatch %Lines
app/app.go0.00%7 Missing ⚠️
p2p/resourcemanager.go96.55%2 Missing and 2 partials ⚠️
cmd/relay/p2p.go40.00%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4639 +/- ##
==========================================
+ Coverage 58.16% 58.31% +0.15% 
==========================================
Files 247 248 +1 Lines 34056 34180 +124 ==========================================
+ Hits 19807 19933 +126 + Misses 11779 11773 -6 - Partials 2470 2474 +4 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

I'd be cautious of including that, without gathering enough data on our side for amount of opened streams nodes have. That is not only for the happy case where we observe a properly functioning cluster running X amount of validators having Y amount of streams at most, but also in the unhappy case, when cluster connections drop, they reconnect. Also when QUIC is enabled.

Until we have that I'm afraid this might cause more harm than good.

@pinebit

Copy link
Copy Markdown
CollaboratorAuthor

Until we have that I'm afraid this might cause more harm than good.

Agreed on the data gap, and that's why the node path ships default off behind the alpha flag - the numbers below (agent verified) explain why the chosen limits have 10–100× headroom in both happy and reconnect cases, QUIC included, and rcmgr metrics during the alpha can confirm them empirically before promotion.

- One stream per message, closed immediately. Every send in p2p/sender.go opens a stream, writes, reads the response, and closes it via defer s.Close(), with a deadline set to the send timeout. So concurrent streams per peer = messages in flight, and no stream outlives its timeout. There is no long-lived stream accumulation to measure.
- Happy case scale: even a 20k-validator cluster runs a handful of concurrent duties per slot (attester, aggregator, proposer, sync), each producing a few in-flight consensus/parsigex messages per peer — low hundreds of concurrent streams per peer at the extreme, against a 4096 per-cluster-peer limit and 16384 system limit (deliberately fixed above go-libp2p's autoscaled ~3072-on-16GB, so limits don't silently depend on host memory).
- Unhappy case (drop + reconnect): streams die with their connection, so reconnect storms pressure the connection scopes, not stream scopes — and that's exactly what the PR sized for: 32 conns per cluster peer, 64 per IP (whole cluster behind one NAT), transient = 128 to absorb a full-cluster burst, and a custom per-IP rate limiter replacing go-libp2p's default burst-of-16, which would otherwise throttle a NAT'd cluster's reconnect regardless of limits.
- QUIC: the resource manager counts streams transport-agnostically — a native QUIC stream and a yamux-over-TCP stream cost the same. The differences cut in QUIC's favor (no per-connection FD). The one thing QUIC changes is dual-transport peers holding both a TCP and a QUIC connection, which 32 conns/peer covers comfortably.

Sign up for freeto 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.

3 participants

@pinebit@KaloyanTanev