fix(tlsnotary): bake wstcp into the image + publish the proxy port range#949
fix(tlsnotary): bake wstcp into the image + publish the proxy port range#949Shitikyan wants to merge 5 commits into
Conversation
TLSNotary verifications failed with nginx 502 / CloseEvent 1006 because the wstcp websocket→TCP proxy could never be reached, on two layers: 1. The runtime image ships no Rust toolchain, so ensureWstcp()'s on-demand `cargo install wstcp` fallback silently failed and the proxy never spawned. Bake the binary in via a dedicated `rust` build stage and copy it to the exact path proxyManager.ts probes ($HOME/.cargo/bin/wstcp). 2. The proxy binds a dynamic port in 55000-57000 inside the container, but docker-compose published none of them — so nginx forwarding /tlsn/<port>/ to 127.0.0.1:<port> hit a dead upstream. Publish a narrow, localhost-bound, env-configurable window; the allocation range (PORT_MIN/MAX, now env-overridable) and the published range read the same vars so they cannot drift. Manually binding one host port was a band-aid that broke on the next session's different port. This fixes both layers durably. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 36 minutes and 41 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe Docker image now bakes in ChangesTLSNotary wstcp runtime and port wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR updates TLSNotary proxy packaging and port exposure. The main changes are:
Confidence Score: 5/5The change is narrowly scoped to TLSNotary proxy packaging, configuration, and Compose port exposure. No correctness issues were identified in the changed Docker, Compose, or TLSNotary proxy allocation paths.
What T-Rex did
Reviews (4): Last reviewed commit: "fix(tlsnotary): publish the wstcp proxy ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
150-151: 📐 Maintainability & Code Quality | 🔵 TrivialPin the
wstcpversion for reproducible builds.
cargo install wstcpfetches the latest release (currently0.2.1) at build time, making builds non-reproducible. Thewstcpcrate is available on crates.io with versions ranging from0.1.0to0.2.1. To prevent unexpected behavior changes or build failures from upstream updates, pin the specific version and use--locked.♻️ Suggested update
FROM rust:1-slim AS wstcp -RUN cargo install wstcp --root /wstcp +RUN cargo install wstcp --version 0.2.1 --locked --root /wstcp🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 150 - 151, The wstcp install step is unpinned and can pull a changing latest release, so update the Dockerfile’s wstcp build stage to install a specific crates.io version of wstcp and use --locked for reproducible builds. Keep the change focused on the FROM rust:1-slim AS wstcp stage and the cargo install wstcp command so the image always builds against the same crate release.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/tlsnotary/constants.ts`:
- Around line 60-61: `PORT_CONFIG` is duplicated and the allocator is still
using the hardcoded range instead of the env-driven values. Update
`portAllocator.ts` to remove its local `PORT_CONFIG` and import the shared one
from `./constants`, then make `proxyManager.ts` also use `PORT_CONFIG` from
`./constants` so there is a single source of truth for the TLSNotary port range.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 150-151: The wstcp install step is unpinned and can pull a
changing latest release, so update the Dockerfile’s wstcp build stage to install
a specific crates.io version of wstcp and use --locked for reproducible builds.
Keep the change focused on the FROM rust:1-slim AS wstcp stage and the cargo
install wstcp command so the image always builds against the same crate release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93605c23-fd3e-4cf7-ac95-581870d043e0
📒 Files selected for processing (3)
Dockerfiledocker-compose.ymlsrc/features/tlsnotary/constants.ts
…ocation portAllocator.ts kept its own hardcoded PORT_CONFIG (55000-57000) and proxyManager imported PORT_CONFIG from it, so the env-overridable values in constants.ts were never used for allocation: the allocator would walk past the published window (e.g. 55064) and hand nginx an unreachable port, re-creating the 502 after the first 64 allocations. Point both consumers at constants.ts (the env-driven copy) and drop the duplicate. Addresses CodeRabbit/Greptile review on #949. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`cargo install wstcp` pulled the latest release at build time. Pin 0.2.1 and pass --locked so the image always builds the same wstcp and an upstream release can't change behaviour or break the build. Addresses CodeRabbit review on #949. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`--locked` forces wstcp 0.2.1's bundled Cargo.lock, whose pinned deps no longer compile on the current rust:1-slim toolchain (cargo exit 101). Keep the version pin for reproducibility but let cargo resolve compatible dependency versions. Validated: the stage builds and yields wstcp 0.2.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The devnet override replaces the node `ports:` list (`!override`), which dropped the proxy-range publish from the base compose — so devnet verifications would still 502. Add a devnet-specific window (55100-55163, +100 offset from mainnet so the two stacks don't collide on host ports), with the allocation env and the published range driven by the same values. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
TLSNotary verifications (e.g. Discord) fail with nginx 502 /
CloseEvent {code: 1006}on the prover. The notary itself is healthy (/tlsnotary/health→ 200, port 7047) — "connected to verifier" succeeds — but the next hop,wss://<node>/tlsn/<port>/, 502s.Root cause is two layers, both confirmed live on node3:
wstcp can't spawn. The TLSNotary flow spawns a
wstcpwebsocket→TCP proxy on demand (proxyManager.ts). The slim runtime image has no Rust toolchain, soensureWstcp()'scargo install wstcpfallback fails silently — nothing ever binds the proxy port. Confirmed:docker exec demos-node→ nowstcp, nocargo.The proxy port isn't reachable from the host. Even with wstcp, the proxy binds a dynamic port in
55000-57000inside the container, butdocker-compose.ymlpublishes none of that range. nginx forwards/tlsn/<port>/→127.0.0.1:<port>→ dead upstream → 502. Manually binding a single port (55688) is a band-aid that breaks on the next session's different port — which is why this kept recurring.Fix
wstcpinto the image via a dedicatedFROM rust:1-slim AS wstcpstage and copy the binary to the exact pathproxyManager.tsprobes ($HOME/.cargo/bin/wstcp,HOME=/app). No toolchain in the runtime image; the proxy spawns instantly. (Validated: the stage builds,wstcp 0.2.1runs.)PORT_MIN/PORT_MAXare now overridable viaTLSNOTARY_PROXY_PORT_MIN/MAX(defaults unchanged) so the allocation range can be narrowed to a host-publishable window.127.0.0.1:55000-55063, env-driven). The allocation range and the published range read the same vars so they can't drift, and binding to127.0.0.1keeps the ports reachable by the host reverse proxy only, not the public internet.Deploy on node3
demos-nodecontainer so the newports:mapping applies. (Immediate unblock without a rebuild:docker cpa prebuiltwstcpinto/app/.cargo/bin/wstcpand add the port-range mapping.)TLSNOTARY_PROXY_PORT_MIN/MAXenv + the127.0.0.1:<min>-<max>port mapping there.Refs:
docs/runbooks/wstcp-reachability-check.md(this makes the containerized default actually reachable instead of "internal only").🤖 Generated with Claude Code
Summary by CodeRabbit