Skip to content
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.

Sign up for GitHub

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

fix(simapp/v2): failed to start HTTP server on port 8080 conflict #22687

Merged
merged 5 commits into from
Dec 2, 2024
Merged

fix(simapp/v2): failed to start HTTP server on port 8080 conflict #22687

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d5cb12b
fix(test): failed to start HTTP server on port 8080 conflict
mmsqe Nov 28, 2024
9f6d375
revert rest config
mmsqe Nov 29, 2024
488d316
Merge remote-tracking branch 'origin/main' into rest_port
mmsqe Nov 29, 2024
794675a
cleanup
mmsqe Nov 29, 2024
33a74d0
Merge branch 'main' into rest_port
mmsqe Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion simapp/v2/simdv2/cmd/testnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
runtimev2 "cosmossdk.io/runtime/v2"
serverv2 "cosmossdk.io/server/v2"
"cosmossdk.io/server/v2/api/grpc"
"cosmossdk.io/server/v2/api/rest"
"cosmossdk.io/server/v2/cometbft"
"cosmossdk.io/server/v2/store"
banktypes "cosmossdk.io/x/bank/types"
Expand Down Expand Up @@ -184,6 +185,7 @@ func initTestnetFiles[T transaction.Tx](
rpcPort = 26657
apiPort = 1317
grpcPort = 9090
restPort = 8080
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Using fixed port 8080 for REST server may cause port conflicts

Defining restPort as a constant with the value 8080 could lead to port conflicts since port 8080 is commonly used by other services. Consider making the REST port configurable or choosing an uncommon default port to avoid potential conflicts.

)
p2pPortStart := 26656

Expand All @@ -192,6 +194,7 @@ func initTestnetFiles[T transaction.Tx](
for i := 0; i < args.numValidators; i++ {
var portOffset int
grpcConfig := grpc.DefaultConfig()
restConfig := rest.DefaultConfig()
if args.singleMachine {
portOffset = i
p2pPortStart = 16656 // use different start point to not conflict with rpc port
Expand All @@ -205,6 +208,11 @@ func initTestnetFiles[T transaction.Tx](
MaxRecvMsgSize: grpc.DefaultConfig().MaxRecvMsgSize,
MaxSendMsgSize: grpc.DefaultConfig().MaxSendMsgSize,
}

restConfig = &rest.Config{
Enable: true,
Address: fmt.Sprintf("127.0.0.1:%d", restPort+portOffset),
}
Comment on lines +211 to +215
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Potential port conflicts in multi-host configurations

The REST server is configured to listen on 127.0.0.1 at port restPort + portOffset when singleMachine is set to true, which helps avoid port conflicts on a single machine. However, when singleMachine is false, the portOffset is not applied, and all instances will listen on the same restPort (port 8080) on their respective hosts. If multiple instances are running on the same host or if port 8080 is already in use on any host, this could cause port conflicts.

Suggestion: Make the REST port configurable or apply portOffset in multi-host setups

To prevent potential port conflicts in multi-host configurations, consider making the REST port configurable regardless of the singleMachine flag or apply the portOffset in both scenarios.

}

nodeDirName := fmt.Sprintf("%s%d", args.nodeDirPrefix, i)
Expand Down Expand Up @@ -338,7 +346,8 @@ func initTestnetFiles[T transaction.Tx](
cometServer := cometbft.NewWithConfigOptions[T](cometbft.OverwriteDefaultConfigTomlConfig(nodeConfig))
storeServer := &store.Server[T]{}
grpcServer := grpc.NewWithConfigOptions[T](grpc.OverwriteDefaultConfig(grpcConfig))
server := serverv2.NewServer[T](serverCfg, cometServer, storeServer, grpcServer)
restServer := rest.NewWithConfigOptions[T](rest.OverwriteDefaultConfig(restConfig))
server := serverv2.NewServer[T](serverCfg, cometServer, storeServer, grpcServer, restServer)
err = server.WriteConfig(filepath.Join(nodeDir, "config"))
if err != nil {
return err
Expand Down
Loading