Added TLSN proxy service and make Telegram backend host configurable#757
Open
SergeyG-Solicy wants to merge 2 commits intotestnetfrom
Open
Added TLSN proxy service and make Telegram backend host configurable#757SergeyG-Solicy wants to merge 2 commits intotestnetfrom
SergeyG-Solicy wants to merge 2 commits intotestnetfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a TLSNotary WebSocket-to-TCP proxy and compose stack, introduces TLSNotary env examples, and makes Telegram backend host configurable plus small identity-dispatcher updates to recognize additional TLSNotary operation names. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Proxy as "wstcp Proxy\n(runs in container)"
participant Notary as "Notary Service\n(container: notary:7047)"
Client->>Proxy: WebSocket connect to PROXY_PORT
Proxy->>Notary: TCP connect to notary:7047
Client->>Proxy: Send request (TLSNotary protocol)
Proxy->>Notary: Forward TCP payload
Notary-->>Proxy: Response
Proxy-->>Client: Relay over WebSocket
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Review Summary by QodoAdd TLSN proxy service and configurable Telegram backend host
WalkthroughsDescription• Add TLSN proxy service for WebSocket-to-TCP bridging • Make Telegram backend host configurable via environment variable • Support new identity operation aliases for TLSN identity management • Improve TLSNotary Docker Compose setup with proxy service Diagramflowchart LR
A["TLSN_TELEGRAM_BACKEND_HOST<br/>Environment Variable"] -->|"configures"| B["Telegram Backend<br/>Server Address"]
C["wstcp Proxy<br/>Service"] -->|"forwards WebSocket"| D["TLSNotary Notary<br/>Server"]
E["New Identity<br/>Operation Aliases"] -->|"tlsn_identity_assign<br/>tlsn_identity_remove"| F["TLSN Identity<br/>Routines"]
File Changes1. src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts
|
Contributor
Code Review by Qodo
1. Proxy port env mismatch
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tlsnotary/.env.example (1)
9-12: Consider documenting the variable name difference.The root
.env.exampleusesTLSNOTARY_PROXY_PORT(line 76) while this file usesPROXY_PORT. The comment mentions this alignment requirement, but users may be confused by the different names. Consider adding a brief note explaining why the naming differs (e.g., this is for Docker Compose interpolation vs. node application config).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tlsnotary/.env.example` around lines 9 - 12, The comment in the TLS notary env uses PROXY_PORT but the root .env.example uses TLSNOTARY_PROXY_PORT which can confuse users; update the comment near PROXY_PORT to explicitly state the variable name difference (e.g., "Note: root .env uses TLSNOTARY_PROXY_PORT while this file uses PROXY_PORT for Docker Compose interpolation / node app config") and explain that they must match at runtime (TLSNOTARY_PROXY_PORT ↔ PROXY_PORT) so users know to keep values in sync.tlsnotary/docker-compose.yml (1)
27-42: Proxy service configuration looks good.The service correctly depends on
notary, uses Docker DNS (notary:7047) to forward traffic, and shares thetlsnnetwork. The YAMLlint warning about redundant quotes on line 37 is a false positive—the colon innotary:7047requires quoting for unambiguous parsing.Consider adding a healthcheck for improved observability in production:
♻️ Optional healthcheck addition
proxy: container_name: tlsn-proxy-${PROXY_PORT:-55688} build: context: . dockerfile: Dockerfile.proxy ports: - "${PROXY_PORT:-55688}:55688" command: ["notary:7047"] restart: unless-stopped depends_on: - notary networks: - tlsn + healthcheck: + test: ["CMD-SHELL", "nc -z localhost 55688 || exit 1"] + interval: 30s + timeout: 5s + retries: 3Note: This requires adding
netcat-openbsdto the Dockerfile.proxy runtime image.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tlsnotary/docker-compose.yml` around lines 27 - 42, Add a container healthcheck to the proxy service to improve observability: in the proxy service block (service name "proxy" and its command "notary:7047") add a healthcheck that probes the notary TCP port (e.g., using nc) and tune interval/retries; also update Dockerfile.proxy to install netcat-openbsd (or another lightweight probe tool) in the runtime image so the healthcheck command is available. Ensure the healthcheck uses the internal DNS target "notary:7047" and set sensible timeout/interval/retries for production.src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
1810-1820: Document the non-Docker deployment requirement forTLSN_TELEGRAM_BACKEND_HOST.The static property
TLSN_EXPECTED_ENDPOINTSinitializes withprocess.env.TLSN_TELEGRAM_BACKEND_HOST ?? "telegram-backend"at module load time. The environment variable is documented in.env.examplebut lacks clarification that"telegram-backend"is an internal service name that requires explicit configuration outside containerized deployments. Add a note to.env.exampleor deployment documentation specifying thatTLSN_TELEGRAM_BACKEND_HOSTmust be explicitly set for non-Docker environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts` around lines 1810 - 1820, The TLSN_EXPECTED_ENDPOINTS static map uses process.env.TLSN_TELEGRAM_BACKEND_HOST ?? "telegram-backend" at module load, so the default "telegram-backend" is an internal Docker service name; update documentation by adding a clear note to .env.example (or deployment docs) that TLSN_TELEGRAM_BACKEND_HOST must be explicitly set to the reachable host/FQDN when running outside Docker (e.g., on developer machines or production VMs), include an example value and a short explanation referencing the TLSN_EXPECTED_ENDPOINTS constant and the TLSN_TELEGRAM_BACKEND_HOST env var so operators know to override the default for non-containerized deployments.tlsnotary/Dockerfile.proxy (1)
14-14: Pin the wstcp version for reproducible builds.
cargo install wstcpinstalls the latest version, which can change between builds and lead to unexpected behavior when rebuilding the image. Pin to a specific version for reproducibility.♻️ Proposed fix
-RUN cargo install wstcp +RUN cargo install wstcp --version 0.2.1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tlsnotary/Dockerfile.proxy` at line 14, The Dockerfile's RUN line installs wstcp without a version, causing non-reproducible builds; update the RUN command that currently reads "cargo install wstcp" to pin a specific release (use cargo install --version "x.y.z" with the chosen semver) and include --locked to ensure Cargo.lock is respected, so replace the bare install in the Dockerfile.proxy with a version-pinned, locked cargo install invocation for wstcp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tlsnotary/Dockerfile.proxy`:
- Around line 16-28: Add a non-root user and switch to it in the proxy image:
create a dedicated user/group (e.g., tlsproxy), chown the copied binary
(/usr/local/bin/wstcp) and any needed certs to that user, and add a USER
tlsproxy line before ENTRYPOINT/CMD so the container no longer runs as root;
ensure the user has a home and a non-login shell as needed and that file
permissions are adjusted so wstcp is executable by that user.
---
Nitpick comments:
In `@src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts`:
- Around line 1810-1820: The TLSN_EXPECTED_ENDPOINTS static map uses
process.env.TLSN_TELEGRAM_BACKEND_HOST ?? "telegram-backend" at module load, so
the default "telegram-backend" is an internal Docker service name; update
documentation by adding a clear note to .env.example (or deployment docs) that
TLSN_TELEGRAM_BACKEND_HOST must be explicitly set to the reachable host/FQDN
when running outside Docker (e.g., on developer machines or production VMs),
include an example value and a short explanation referencing the
TLSN_EXPECTED_ENDPOINTS constant and the TLSN_TELEGRAM_BACKEND_HOST env var so
operators know to override the default for non-containerized deployments.
In `@tlsnotary/.env.example`:
- Around line 9-12: The comment in the TLS notary env uses PROXY_PORT but the
root .env.example uses TLSNOTARY_PROXY_PORT which can confuse users; update the
comment near PROXY_PORT to explicitly state the variable name difference (e.g.,
"Note: root .env uses TLSNOTARY_PROXY_PORT while this file uses PROXY_PORT for
Docker Compose interpolation / node app config") and explain that they must
match at runtime (TLSNOTARY_PROXY_PORT ↔ PROXY_PORT) so users know to keep
values in sync.
In `@tlsnotary/docker-compose.yml`:
- Around line 27-42: Add a container healthcheck to the proxy service to improve
observability: in the proxy service block (service name "proxy" and its command
"notary:7047") add a healthcheck that probes the notary TCP port (e.g., using
nc) and tune interval/retries; also update Dockerfile.proxy to install
netcat-openbsd (or another lightweight probe tool) in the runtime image so the
healthcheck command is available. Ensure the healthcheck uses the internal DNS
target "notary:7047" and set sensible timeout/interval/retries for production.
In `@tlsnotary/Dockerfile.proxy`:
- Line 14: The Dockerfile's RUN line installs wstcp without a version, causing
non-reproducible builds; update the RUN command that currently reads "cargo
install wstcp" to pin a specific release (use cargo install --version "x.y.z"
with the chosen semver) and include --locked to ensure Cargo.lock is respected,
so replace the bare install in the Dockerfile.proxy with a version-pinned,
locked cargo install invocation for wstcp.
🪄 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: d4e5d0bd-5876-466c-ad91-866ce657030a
📒 Files selected for processing (5)
.env.examplesrc/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.tstlsnotary/.env.exampletlsnotary/Dockerfile.proxytlsnotary/docker-compose.yml
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+27
to
+33
| proxy: | ||
| container_name: tlsn-proxy-${PROXY_PORT:-55688} | ||
| build: | ||
| context: . | ||
| dockerfile: Dockerfile.proxy | ||
| ports: | ||
| - "${PROXY_PORT:-55688}:55688" |
Contributor
There was a problem hiding this comment.
1. Proxy port env mismatch 🐞 Bug ≡ Correctness
tlsnotary/docker-compose.yml exposes the proxy on PROXY_PORT, but the node’s tlsnotary.getInfo endpoint advertises the proxy using TLSNOTARY_PROXY_PORT; if they diverge, SDK clients will receive an incorrect proxyUrl and fail to connect. This is easy to misconfigure because the two variables are named differently and live in different .env files.
Agent Prompt
### Issue description
The TLSNotary proxy port is configured with `PROXY_PORT` in `tlsnotary/docker-compose.yml`, but the node advertises the proxy endpoint using `TLSNOTARY_PROXY_PORT`. If these values differ, clients will attempt to connect to the wrong port.
### Issue Context
- Docker compose controls what port is actually exposed on the host.
- `tlsnotary.getInfo` is used for SDK auto-configuration and must return the real reachable proxy port.
### Fix Focus Areas
- Make docker-compose use `TLSNOTARY_PROXY_PORT` (or make the node read `PROXY_PORT` too), so there is a single source of truth.
- Update the tlsnotary `.env.example` accordingly.
- tlsnotary/docker-compose.yml[27-37]
- tlsnotary/.env.example[6-12]
- src/libs/network/manageNodeCall.ts[643-670]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
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
1 participant
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 by CodeRabbit
New Features
Chores