[gosunuts]

Public Esplora/REST connections currently consume the same pre-authentication connection budget as management JSON-RPC. Add opt-in --public-bind so public HTTP traffic has a separate bounded listener and cannot consume management connection permits.

When enabled, /api, /esplora and enabled REST routes move to the public port; --rpc-bind serves authenticated JSON-RPC only. The public listener closes JSON-RPC requests with 404 before authentication or body upload. Without this option, the combined listener, routes and 128-connection default remain unchanged.

• Add independent positive --rpc-max-connections and --public-max-connections limits (128 each), with CLI/environment/TOML resolution through the existing option table.
• Reuse the existing connection permits, per-source bounds, timeouts and dispatcher quotas. Both bindings share one Context and its authoritative chain, mempool, indexes and broadcast gateway; each binding retains its own Handler cancellation latch and blocking quota.
• Bind both sockets before starting listeners, record every spawned listener in NodeServices, and join both through existing startup rollback and shutdown. Preserve existing RpcServer construction and serving APIs.
• Expose bounded-label occupancy, rejection and request-duration metrics, and update API-34, WF-02, embedding and operator documentation. Public mode retains both existing Esplora namespaces, including backend helpers and transaction broadcast.

Validation on Windows/MSVC, native validation with fjall:

• RPC server tests: 35 passed, including public keep-alive plus incomplete-head saturation, successful authenticated management RPC, public rejection/recovery, finite management admission, independent cancellation and closed listener routes.
• Existing HTTP authentication/CORS integration tests: 21 passed.
• Layered configuration tests: 25 passed, including wildcard bind conflicts, distinct concrete addresses and simultaneous ephemeral binds; CLI precedence regression passed.
• Node startup/shutdown and public-bind rollback regressions passed, including socket release and reopening node state after failed startup.
• Standalone template-provider lockfile includes the RPC metrics dependency; its cargo metadata --locked --offline --format-version 1 check passed.
• Existing chain_feed::tests::unsettled_or_failed_chain_fences_never_publish_a_stable_hint passed.
• cargo fmt --all -- --check and Clippy for rpc/node/binary with fjall, all targets and -D warnings passed.

Sync impact: this changes transport admission and service composition, not authoritative synchronization, validation or commit paths. SYNC-03 is relevant to bounded optional RPC resources; existing Context capability, stable-read, gateway and lock-order boundaries are retained. The existing chain-fence proof above exercises refusal during unsettled/failed transitions. These tests establish connection-capacity isolation only: CPU, process descriptor limits, storage and lock contention remain shared, and foreground progress under resource exhaustion is not claimed. Existing process/reference/performance campaigns remain in their owning CI lanes.

Fixes #1586.

Confidence Score: 5/5

The changes since the previous review appear safe to merge.

Summary

The PR adds an optional public HTTP listener with a separate connection limit.

• Public HTTP connections get their own listener and capacity.

No new actionable issues were found in the changes since the previous review. No previous review threads were supplied.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Config[Resolved configuration] --> Bind[Bind both sockets before starting listeners]
  Bind --> Management[Management listener: authenticated JSON-RPC]
  Bind --> Public[Public listener: Esplora and enabled REST]
  Management --> Context[Shared Context and node services]
  Public --> Context
  Services[NodeServices] --> Management
  Services --> Public
  Services --> Shutdown[Stop and join both listeners]

Loading

_{Reviews (2) · Last reviewed commit: "fix(rpc): address split-listener review ..." · Reviewed by Greptile}

[amazon-q-developer]

Review Summary

This PR correctly implements the --public-bind feature to isolate public HTTP traffic (Esplora/REST) from the management JSON-RPC listener with independent connection budgets. The core architecture — RpcSurface, RpcBindings, the surface-aware accept loop, route filtering before authentication/body upload, independent ConnectionPermit per surface, and the RequestTimer metrics — is well-designed and coherent.

The startup rollback path (binding management first, then public; StartupGuard teardown releasing both) is correctly implemented and verified by the new lifecycle tests.

Findings

Potentially blocking:

• config.rs: The public-bind same-address validation exempts port 0, but does so even when both addresses use port 0. Two TcpListener::bind("0.0.0.0:0") calls could receive the same OS-assigned port, causing a runtime bind failure instead of a useful validation error. See inline comment.

Non-blocking observations:

• lifecycle.rs: The public listener is wired with the management rpc_auth object. Since the public surface filter blocks all JsonRpc routes before authentication, this is inert — but it creates an implicit coupling that could silently authenticate management credentials if the surface boundary ever changes. Worth documenting or structurally separating.
• server.rs: The set_write_timeout failure before writing the 503-busy response propagates as an error, which means the busy response is silently dropped and the error is logged as a generic "connection setup failed". This is minor but worth being aware of.

All 35 RPC server tests, the lifecycle tests (split_rpc_listeners_start_and_join_through_node_services, public_bind_failure_rolls_back_management_binding_and_node_state), and the existing integration tests pass per the PR description. The implementation correctly preserves the existing combined-listener behavior when --public-bind is not set.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[cubic-dev-ai]

All reported issues were addressed across 18 files

<sub>Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/node/src/config.rs">

<violation number="1" location="crates/node/src/config.rs:391">
P2: This rejects distinct IPv6 link-local listeners that use the same address on different interfaces because `ip()` drops the scope ID. Account for distinct nonzero scope IDs when checking link-local addresses.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This rejects distinct IPv6 link-local listeners that use the same address on different interfaces because ip() drops the scope ID. Account for distinct nonzero scope IDs when checking link-local addresses.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/node/src/config.rs, line 391:

<comment>This rejects distinct IPv6 link-local listeners that use the same address on different interfaces because `ip()` drops the scope ID. Account for distinct nonzero scope IDs when checking link-local addresses.</comment>

<file context>
@@ -380,10 +380,20 @@ impl NodeConfig {
+                // Cross-family dual-stack conflicts remain OS-checked at bind.
+                address.port() == 0
+                    || address.port() != management.port()
+                    || (address.ip() != management.ip()
+                        && (address.is_ipv4() != management.is_ipv4()
+                            || (!address.ip().is_unspecified()
</file context>

Suggested change

	|| (address.ip() != management.ip()
	&& (address.is_ipv4() != management.is_ipv4()
	|| (!address.ip().is_unspecified()
	&& !management.ip().is_unspecified())))
	|| ((address.ip() != management.ip()
	|| matches!(
	(address, management),
	(SocketAddr::V6(public), SocketAddr::V6(management))
	if public.ip().is_unicast_link_local()
	&& public.scope_id() != 0
	&& management.scope_id() != 0
	&& public.scope_id() != management.scope_id()
	))
	&& (address.is_ipv4() != management.is_ipv4()
	|| (!address.ip().is_unspecified()
	&& !management.ip().is_unspecified())))