Consensus CLI should reject execution WebSocket endpoints when IPC transport is selected

[Kewe63]

Summary

The consensus CLI allows a complete IPC transport configuration to be combined with --execution-ws-endpoint, even though the WebSocket endpoint is an RPC-only option.

The mixed configuration passes StartCmd::validate(). Later, StartConfig::engine_config() prioritizes IPC and constructs EngineConfig::Ipc, causing the explicitly supplied WebSocket endpoint to be silently ignored.

The CLI should reject this IPC/RPC conflict during startup validation instead of accepting a configuration it cannot fully honor.

Affected files

• crates/malachite-cli/src/cmd/start.rs
• crates/malachite-app/src/config.rs

The validation gap is in StartCmd::validate() in crates/malachite-cli/src/cmd/start.rs. StartConfig::engine_config() in crates/malachite-app/src/config.rs demonstrates the resulting behavior.

Observed behavior

StartCmd::validate() determines whether IPC options are present with:

let has_ipc_options = self.eth_socket.is_some() || self.execution_socket.is_some();

It determines whether RPC options are present with:

let has_rpc_options = self.eth_rpc_endpoint.is_some()
    || self.execution_endpoint.is_some()
    || self.execution_jwt.is_some();

execution_ws_endpoint is missing from has_rpc_options.

However, the same method explicitly classifies it as part of the RPC transport immediately afterward:

let uses_rpc_transport = self.eth_rpc_endpoint.is_some()
    || self.execution_endpoint.is_some()
    || self.execution_ws_endpoint.is_some()
    || self.execution_jwt.is_some();

This mixed configuration is therefore accepted:

--eth-socket /tmp/reth.ipc
--execution-socket /tmp/reth-auth.ipc
--execution-ws-endpoint ws://localhost:8546

StartConfig::engine_config() checks for a complete IPC configuration first. It consequently selects EngineConfig::Ipc, and execution_ws_endpoint is not included in that configuration.

Expected behavior

Any IPC transport option should conflict with any RPC transport option.

Because --execution-ws-endpoint is documented and handled as RPC-only, it should be included in has_rpc_options. Combining it with IPC options should fail with the existing IPC/RPC conflict error.

The conflict message's RPC option list should also include --execution-ws-endpoint.

Reproduction

The following focused regression test was run against current main:

#[test]
fn validate_err_when_mixing_ipc_and_execution_ws_endpoint() {
    let mut cmd = new_start_cmd();
    cmd.eth_socket = Some("/tmp/reth.ipc".to_string());
    cmd.execution_socket = Some("/tmp/reth-auth.ipc".to_string());
    cmd.execution_ws_endpoint = Some(dummy_url());

    assert!(
        cmd.validate().is_err(),
        "--execution-ws-endpoint is an RPC option and must conflict with IPC"
    );
}

Command:

cargo +1.94.0 test \
  -p arc-node-consensus-cli \
  validate_err_when_mixing_ipc_and_execution_ws_endpoint \
  -- --nocapture

Result on the pre-fix implementation:

test cmd::start::tests::validate_err_when_mixing_ipc_and_execution_ws_endpoint ... FAILED

The test was executed against commit:

97f8da0dc4faa703fe2d68ca007e40dab2c8a9ef

Root cause

The IPC/RPC conflict detection and RPC transport detection use different option sets.

execution_ws_endpoint is included in uses_rpc_transport but omitted from has_rpc_options. This allows it to be recognized as an RPC transport option without triggering the IPC/RPC conflict check.

Why this matters

Silently accepting and ignoring an explicit CLI argument can mislead operators. An operator may believe the configured WebSocket endpoint is being used for execution-layer events while the node has actually selected IPC and discarded the WebSocket setting.

Failing during validation would expose the configuration mistake immediately and ensure explicitly supplied options are not silently ignored.

Suggested fix

Include execution_ws_endpoint in has_rpc_options:

let has_rpc_options = self.eth_rpc_endpoint.is_some()
    || self.execution_endpoint.is_some()
    || self.execution_ws_endpoint.is_some()
    || self.execution_jwt.is_some();

Also include --execution-ws-endpoint in the existing conflict error's RPC option list.

No transport-selection behavior should need to change.

Potential regression tests

Verify that:

• IPC configuration without RPC options remains valid;
• RPC configuration with --execution-ws-endpoint remains valid;
• IPC combined with --execution-ws-endpoint is rejected;
• the returned error identifies the IPC/RPC conflict and lists --execution-ws-endpoint.

Duplicate check

Open and closed issues and pull requests were searched using:

• execution-ws-endpoint
• execution_ws_endpoint
• execution websocket with eth-socket
• Conflicting options detected
• IPC and RPC

No prior direct duplicate was found. Issue #294 and PR #295 concern separated-host deprecation wording and do not address mixed IPC/WebSocket validation or the silently ignored endpoint.

[osr21]

Confirmed against 97f8da0. The premise holds exactly as written, and #337 is a faithful implementation of it. Source review only — no Rust toolchain available here, so I traced call paths rather than running your test. Three things worth adding: one that strengthens the report, one that calibrates its severity down, and one upgrade-compatibility caveat I'd want in the PR before it merges.

The silent-drop is real

StartConfig::engine_config() (crates/malachite-app/src/config.rs:159) checks the IPC pair first and builds:

Some(EngineConfig::Ipc(EthIpcConfig { eth_socket, execution_socket }))

EthIpcConfig has exactly those two fields — there is no ws slot to carry the value into, so execution_ws_endpoint isn't merely deprioritized, it is structurally unrepresentable in the IPC branch. Only the RPC branch reads it (:172, with the derive_ws_url fallback). Your root-cause diagnosis is correct.

Stronger than you argued: the mixed config violates an invariant the test suite already asserts

Because execution_ws_endpoint is in uses_rpc_transport (:712), the mixed config sets that flag true and the node emits the deprecation warning — which tells the operator:

Migrate to IPC: on the consensus layer use --eth-socket and --execution-socket …

…to an operator who has just passed exactly --eth-socket and --execution-socket. So today's behaviour isn't only "silently ignores a flag", it's "actively instructs you to do the thing you already did."

That matters because validate_no_deprecation_warning_in_ipc_mode (start.rs:918) exists specifically to assert:

assert!(!logs_contain("deprecated"), "IPC mode must not emit the RPC deprecation warning");

The invariant is already written down — the test just doesn't exercise the ws case, so the gap slipped through. This reframes the issue from "missing conflict check" to "an existing, explicitly tested invariant is violated on an uncovered input," which is a considerably stronger case for merging. #337 fixes it as a side effect, since validation now returns Err before reaching the warning.

Severity calibration: no events are actually lost

The "Why this matters" section says an operator "may believe the configured WebSocket endpoint is being used for execution-layer events." Worth being precise about what actually happens, because it's better than implied. SubscriptionEndpoint (crates/eth-engine/src/engine.rs:53) has two variants:

pub enum SubscriptionEndpoint {
    Ipc { socket_path: String },
    Ws { url: Url },
}

Engine::new_ipc unconditionally builds SubscriptionEndpoint::Ipc { socket_path: eth_socket }, while Engine::new_rpc builds ws_endpoint.map(|url| SubscriptionEndpoint::Ws { url }). So in IPC mode the subscription the ws flag exists to enable is still established — over the eth socket. The operator loses no functionality; they lose an accurate mental model of which transport carries the events, which has real consequences for firewalling, monitoring and debugging, but is not event loss. I'd call this a correctness-of-configuration bug, not a functional one, and I think saying so plainly makes the issue more likely to be accepted rather than less.

The mirror-image case is arguably worse and is not covered here

In the RPC branch sub_endpoint is an Option and can legitimately end up None: derive_ws_url returns None when the URL has no explicit port (the repo's own derive_ws_url_returns_none_without_port test pins this). So:

--eth-rpc-endpoint http://host  --execution-endpoint …  --execution-jwt …

with no explicit --execution-ws-endpoint yields no subscription endpoint at all — silently, and with no validation error. That is the same failure mode you're arguing against ("explicitly supplied options must not be silently ignored") in its harsher form: a required capability silently absent rather than a redundant flag silently dropped. It's out of scope for #337 and I'd keep it out, but it's worth a separate issue.

On #337

I checked the diff against every existing test in the module and it is non-breaking:

• validate_warns_on_execution_ws_endpoint_alone (:905) — ws alone now sets has_rpc_options, but has_ipc_options is false, so it still returns Ok and still warns. Passes.
• validate_no_deprecation_warning_in_ipc_mode (:918) — ws is None, unaffected.
• validate_ok_with_only_rpc_options, validate_err_when_mixing_ipc_and_rpc, validate_err_with_another_mix_of_options — unaffected.

Two suggestions:

1. Collapse the two lists instead of extending one. The root cause is two hand-maintained enumerations of "what counts as an RPC option" that drifted apart. After #337 they are character-for-character identical — both are eth_rpc_endpoint || execution_endpoint || execution_ws_endpoint || execution_jwt. Adding a fourth item to one list fixes this instance while preserving the exact structure that produced it. Since they now agree, they can simply be one binding:

let has_rpc_options = self.eth_rpc_endpoint.is_some()
    || self.execution_endpoint.is_some()
    || self.execution_ws_endpoint.is_some()
    || self.execution_jwt.is_some();

// … conflict check …

if has_rpc_options {   // was: uses_rpc_transport

That makes a future divergence impossible rather than merely fixed, and it's strictly smaller than the current diff.

2. Add the full-RPC-plus-ws case. Your own regression list includes "RPC configuration with --execution-ws-endpoint remains valid," but the only existing coverage is the alone form. The realistic production shape — eth_rpc_endpoint + execution_endpoint + execution_jwt + execution_ws_endpoint — isn't asserted anywhere, and it's precisely what a careless future refactor of has_rpc_options would break.

Upgrade-compatibility caveat worth calling out in the PR

This turns a previously accepted configuration into a hard startup failure, and there is a plausible population of configs in exactly that state. start.rs:659 sits in the deployment flag-reconstruction block (--- Deployment-specific fields (set by quake …) ---) and emits --execution-ws-endpoint unconditionally when set, right beside --eth-socket and --execution-socket. Combined with the active RPC→IPC deprecation push, an operator who migrated by adding the two socket flags and leaving a now-inert --execution-ws-endpoint in their manifest has a node that starts fine today and will refuse to start after this change.

That's the correct fail-fast outcome and I'm not arguing against it — but it's a breaking config change rather than a pure bug fix, so it warrants a release-note line, and the error message could earn its keep by saying the ws endpoint is unused in IPC mode and can simply be removed. The current generic "choose either IPC or RPC" wording doesn't tell that operator that deleting one line is the whole fix.

Nicely scoped report — the has_rpc_options vs uses_rpc_transport diff is exactly the right place to point, and pinning it to a named commit made it quick to confirm.