Transaction relay configuration should reject non-HTTP upstream URLs

[Kewe63]

Summary

The --arc.tx.relays configuration currently accepts any URL that can be parsed by url::Url, including schemes that cannot be used by the transaction relay's HTTP transport.

For example, the following value is accepted during startup validation:

file:///tmp/upstream

The value is later passed to an explicitly HTTP-based RPC client. This allows the node to start with an unusable relay configuration instead of rejecting it immediately with a clear configuration error.

---

Affected Files

crates/node/src/main.rs
crates/evm-node/src/rpc_middleware.rs

The validation gap is in build_tx_relays() in crates/node/src/main.rs.

---

Observed Behavior

build_tx_relays() currently validates each configured relay by calling:

rust
url::Url::parse(s)

This only establishes that the input is a syntactically valid URL. It does not verify that its scheme is supported by the downstream transport.

As a result, this configuration is accepted:

--arc.tx.relays file:///tmp/upstream

The returned relay list contains:

["file:///tmp/upstream"]

However, the relay implementation later constructs each client with:

rust
RpcClient::new_http_with_client(...)

using a reqwest::Client. Therefore, a file:// URL cannot function as a transaction relay upstream.

The invalid configuration is detected only when forwarding is attempted, where it becomes a transport/failover error rather than a startup validation error.

---

Expected Behavior

--arc.tx.relays should only accept URL schemes supported by the transaction relay transport:

http
https

Unsupported schemes such as the following should be rejected during startup:

file://
ftp://
ws://
wss://

The error should identify the invalid --arc.tx.relays entry and explain that an HTTP or HTTPS URL is required.

---

Reproduction

I added the following focused regression test against current main:

rust
#[test]
fn test_build_tx_relays_rejects_non_http_url() {
let err = tx_relays_from_args(&[
"--arc.tx.relays",
"file:///tmp/upstream",
])
.expect_err("transaction relay upstreams must use HTTP or HTTPS");

assert!(
    err.to_string()
        .contains("invalid --arc.tx.relays entry")
);

}

The test fails because the current implementation returns the URL as a valid relay configuration:

thread 'tests::test_build_tx_relays_rejects_non_http_url' panicked:

transaction relay upstreams must use HTTP or HTTPS:
["file:///tmp/upstream"]

test tests::test_build_tx_relays_rejects_non_http_url ... FAILED

The test was executed against:

97f8da0

which was the current main SHA at the time of verification.

---

Root Cause

The validation boundary checks URL syntax but not transport compatibility:

rust
url::Url::parse(s)
.map(|_| s.to_string())

The downstream implementation assumes an HTTP transport:

rust
RpcClient::new_http_with_client(http.clone(), url)

Therefore, the validation contract is weaker than the runtime transport contract.

---

Why This Matters

Invalid relay configuration should fail before the node starts.

Accepting unsupported schemes can cause:

• a node to appear correctly configured at startup
• transaction forwarding to fail only when it is first needed
• unnecessary failover attempts
• misleading relay exhaustion or generic transport errors
• longer diagnosis time for an operator configuration error

This is especially relevant because --arc.tx.relays is a prioritized list: one unsupported entry can introduce avoidable failures before a working upstream is selected.

---

Suggested Fix

After parsing each URL, validate its scheme:

rust
let url = url::Url::parse(s)
.map_err(|e| eyre::eyre!(
"invalid --arc.tx.relays entry {s:?}: {e}"
))?;

if !matches!(url.scheme(), "http" | "https") {
return Err(eyre::eyre!(
"invalid --arc.tx.relays entry {s:?}:
transaction relay upstreams must use HTTP or HTTPS"
));
}

The existing ordering and trimmed string behavior can otherwise remain unchanged.

---

Potential Regression Tests

Add focused tests verifying that:

• http://a:8545 is accepted
• https://a.example is accepted
• file:///tmp/upstream is rejected
• ftp://a.example is rejected
• malformed URLs remain rejected
• multiple HTTP/HTTPS relay URLs preserve their configured order

---

Scope

This should be a small fail-fast configuration fix.

It does not require changing:

• transaction forwarding behavior
• failover ordering
• request timeout handling
• the RPC client implementation
• protocol or consensus behavior

---

Duplicate Check

I searched open and closed issues and pull requests using:

• arc.tx.relays
• transaction relay URL scheme
• TxRelays HTTP upstream
• file:// relay
• new_http_with_client

I did not find a direct duplicate or an existing PR implementing this validation.

Issue #59 concerns the older singular --rpc.forwarder path and does not cover --arc.tx.relays scheme validation.

[osr21]

Traced this against 97f8da0 (the SHA you tested) and the premise holds — with a slightly sharper mechanism than the issue describes, and one consequence I think it understates. Source review only; no Rust toolchain here, so I verified structurally rather than by running your test.

The gap is real, and the code already documents the contract you're strengthening

The downstream consumer is TxRelays::new (crates/evm-node/src/rpc_middleware.rs:512), and its doc comment states the contract explicitly:

URLs are pre-validated at the CLI boundary; any that fail to parse here are skipped.

So the inner layer guards parse failures only and delegates everything else upstream — which is exactly the contract build_tx_relays currently under-delivers on. Following file:///tmp/upstream through:

1. reqwest::Url::parse("file:///tmp/upstream") succeeds, so the filter_map at :522 does not skip it.
2. RpcClient::new_http_with_client(http.clone(), url) returns RpcClient — not Result<RpcClient>, and it's used inside Some(...) with no ?. Construction is infallible, so a file:// client is genuinely built and stored at startup.
3. The failure therefore cannot surface before the first forward. Your "detected only when forwarding is attempted" is structurally guaranteed, not incidental.
4. forward() then advances past upstreams that fail with "a transport error, timeout, or HTTP 5xx/429" — so the bad entry is consumed as a failover step, producing exactly the misleading relay-exhaustion signature you predict.

One consequence worth adding to the issue

TxRelays::new ends with:

(!clients.is_empty()).then(|| Self { .. })

So if every configured relay is dropped, it returns None, tx_relays is None, and relaying is silently disabled — the node runs as though no relays were ever configured. Combined with the filter_map's silent skip (a warn! at most), the failure mode for a misconfigured relay list isn't just "fails late," it's "can degrade to no relaying at all, with the node reporting healthy." That's a stronger argument for fail-fast than the one in the issue.

#336 looks correct, and the scheme check has no hole

Since rust-url and Node's URL both implement the WHATWG URL Standard, I ran the tricky inputs through the same parser semantics url::Url::parse uses:

input	parses	scheme	normalizes to	#336
http://a:8545	yes	http	http://a:8545/	accepts
HTTP://A:8545	yes	http	http://a:8545/	accepts
http:a:8545	yes	http	http://a:8545/	accepts
http:///tmp/upstream	yes	http	http://tmp/upstream	accepts
http://[::1]:8545	yes	http	http://[::1]:8545/	accepts
http:// / https://	no	—	—	already rejected
http://a:99999	no	—	—	already rejected
file: ftp: ws: wss:	yes	—	—	rejected

The two odd ones are worth calling out because they look like holes and aren't: http:///tmp/upstream collapses the extra slash to host tmp, and http:a:8545 normalizes to http://a:8545/. Both are genuinely HTTP URLs reqwest can drive, so accepting them is correct. Empty-host and out-of-range-port cases never reach the scheme check — Url::parse already rejects them. matches!(url.scheme(), "http" | "https") is sufficient, and scheme is lowercased during parsing so uppercase input isn't falsely rejected.

It also matches house style: derive_ws_url in crates/malachite-app/src/config.rs already branches on "http" | "https" with _ => for everything else.

Two suggestions for #336

1. Update the TxRelays::new doc comment. It's the written contract between the two layers, and #336 upgrades the guarantee from "parses" to "parses and is http/https". Leaving it saying only "fail to parse here are skipped" is precisely how someone reintroduces this gap later — the comment is what a future reader will trust instead of re-deriving the CLI validation.

2. Pin the mixed valid/invalid case. Your "Potential Regression Tests" list is otherwise fully covered by #336, but nothing asserts what happens to a list like:

--arc.tx.relays http://good:8545,file:///bad

Because build_tx_relays collects into eyre::Result<Vec<String>>, collect() short-circuits on the first Err, so the whole list is rejected rather than the bad entry being dropped and the good one kept. That's the correct behaviour and it's the single thing that most distinguishes fail-fast from the old silent-skip path — worth a test, since a future refactor to filter_map-style leniency would silently pass every test currently in the PR.

On the #59 scoping

Confirmed. rpc_forwarder appears in crates/node/src/main.rs only as a clap conflicts_with = "rpc_forwarder" reference at :307 — it's Reth's own flag, not an arc-node-owned validation site. So there's no parallel build_* boundary in this repo that should get the same treatment, and keeping #335 scoped to --arc.tx.relays is right.

Well-researched issue — the duplicate check and the pre-verified reproduction against a named SHA made this quick to confirm.