-
Notifications
You must be signed in to change notification settings - Fork 43
ton local fixes #2339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ton local fixes #2339
Conversation
|
👋 tt-cll, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes Docker networking issues for TON blockchain nodes running in local containers. Previously, nodes couldn't connect using InternalHTTPUrl because the TON chain was on its own isolated network, and the server URL exposed the host port instead of the container port.
Key changes:
- Removed creation of isolated Docker network for TON containers
- Changed port mapping to use container ports directly instead of mapping to different host ports
- Updated environment variables to use dynamic port values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fmt.Sprintf("%s:%s/tcp", ports.HTTPServer, ports.HTTPServer), | ||
| fmt.Sprintf("%s:%s/tcp", ports.LiteServer, ports.LiteServer), |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Port mapping now uses the same port for both host and container (e.g., 'hostPort:containerPort' becomes 'port:port'). This may cause port conflicts if multiple TON instances run on the same host, as they would attempt to bind to the same host ports. Consider whether this is the intended behavior or if dynamic host port allocation would be more appropriate.
| fmt.Sprintf("%s:%s/tcp", ports.HTTPServer, ports.HTTPServer), | |
| fmt.Sprintf("%s:%s/tcp", ports.LiteServer, ports.LiteServer), | |
| fmt.Sprintf("%s/tcp", ports.HTTPServer), | |
| fmt.Sprintf("%s/tcp", ports.LiteServer), |
e1a9b5d
Nodes on a local docker network couldn't connect using the
InternalHTTPUrlbecause the ton chain was on its own network and the server url being exposed was the host port and not the container portBelow is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.
Why
The changes streamline the TON blockchain component configuration process in the testing framework, removing unused network setup and directly utilizing port mappings. Also, they align environment variable usage with dynamic port assignments, enhancing the flexibility of TON node setups for testing purposes.
What
github.com/testcontainers/testcontainers-go/network, simplifying the setup by not explicitly creating a new network.defaultLiteServerPortconstant as the lite server port is now dynamically assigned.baseEnvmap to useports.HTTPServerandports.LiteServerfor setting environment variables, allowing for dynamic port configuration based on input.networkNamedirectly toframework.DefaultNetworkName, removing the need for creating and naming a new network.reqstruct for container setup:WaitingForconfiguration to use the dynamically assignedports.LiteServer.ExternalHTTPUrlandInternalHTTPUrlto use the dynamically assigned lite server port, ensuring the URLs reflect the actual configuration of the test container.