Conversation
kutluhaneth46
approved these changes
Sep 14, 2026
kutluhaneth46
left a comment
There was a problem hiding this comment.
Review
Checked this against current main.
Compose already publishes ${PORT__OP_GETH_WS:-9994}:8546, but scripts/start-op-geth.sh only enables HTTP (--http.addr=0.0.0.0 --http.port=8545) and never turns on the WebSocket server. With op-geth defaulting to --ws=false, host port 9994 has nothing listening — that matches #62.
The three new flags line up with the published mapping:
--ws
--ws.addr=0.0.0.0
--ws.port=8546Good calls on scope:
- Keep default WS API / origin policy instead of copying HTTP's
*CORS/vhosts into WebSocket. - Document plain
ws://vs TLSwss://so operators do not assume TLS on 9994. - Stay out of the broader RPC-hardening work in #88 / #90; this PR only closes the missing listener for #62.
The offline argv tests are a solid regression net for the wrapper. I did not re-run the real-binary handshake locally; the write-up of that check is clear.
LGTM for #62.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #62.
Compose publishes
${PORT__OP_GETH_WS:-9994}:8546, but the op-geth entrypoint never enables the WebSocket server. The configured op-geth v1.101503.4 binary defaults to--ws=false, so HTTP works while the published WebSocket port has no listener.ws://from TLSwss://.Verification
python3 -m unittest discover -s tests -v: all three tests failed against the original entrypoint; all three pass with this change (none skipped on the test host).sh -n scripts/start-op-geth.shandgit diff --cached --check: passed.HTTP/1.1 101 Switching Protocols. An explicit trailing--ws=falsecontrol disabled 8546 again.sha256:618d925cfdbe9310e971dc07e30f655f4da7ac0c1e83bb7dd4f906452dc6e030.Scope and limitations
No live RPC, peer, snapshot service, or existing node data was used. Docker Compose end-to-end startup/port forwarding was not run because Docker was unavailable in the WSL test environment. The committed tests validate wrapper arguments, not binary semantics; the real-binary check above was a separate local verification. Tests skip explicitly if Linux user/network namespaces or required tools are unavailable.
This activates the already-published WS endpoint. Existing Compose host binding, port overrides, HTTP/auth RPC settings, and data handling remain unchanged. Operators should apply their existing network-access controls; this change does not add authentication or TLS.
PRs #88 and #90 also touch RPC host bindings but do not enable the missing WS listener. This change is limited to #62, not those broader changes.
Implementation and test preparation were AI-assisted.