Skip to content

feat: use tls on port 443#65

Merged
vladjdk merged 2 commits intomainfrom
vlad/443-tls
Mar 19, 2026
Merged

feat: use tls on port 443#65
vladjdk merged 2 commits intomainfrom
vlad/443-tls

Conversation

@vladjdk
Copy link
Copy Markdown
Collaborator

@vladjdk vladjdk commented Mar 19, 2026

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR adds TLS support to the gRPC client by detecting whether the target address ends with :443 and switching from insecure.NewCredentials() to credentials.NewTLS(...) accordingly. The change is small and self-contained within NewClient.

  • The port-only heuristic (strings.HasSuffix(grpcAddress, ":443")) works for the standard public-node case but silently degrades to an insecure connection for TLS endpoints on non-standard ports (e.g. 8443).
  • &tls.Config{} should explicitly set MinVersion: tls.VersionTLS12 to pin the minimum TLS version and avoid any ambiguity across Go build versions.

Confidence Score: 4/5

  • Safe to merge for the :443 use-case; minor improvements recommended for robustness and security hygiene.
  • The change is functionally correct for the common case (public Cosmos gRPC endpoints on port 443). No regressions are introduced for existing non-443 endpoints since those continue to use insecure credentials as before. The two flagged issues are best-practice improvements rather than blocking bugs.
  • chains/cosmos/client/client.go — TLS detection heuristic and TLS config hardening.

Important Files Changed

Filename Overview
chains/cosmos/client/client.go Adds conditional TLS for gRPC connections when the address ends with :443; the detection heuristic is fragile (misses TLS on non-443 ports) and the empty tls.Config{} should pin MinVersion explicitly.

Last reviewed commit: "use tls on port 443"

Comment on lines +61 to +65
if strings.HasSuffix(grpcAddress, ":443") {
transportCreds = credentials.NewTLS(&tls.Config{})
} else {
transportCreds = insecure.NewCredentials()
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Port-only TLS detection is fragile

TLS detection is keyed exclusively on :443 as a suffix. This means:

  • Any gRPC endpoint using TLS on a non-standard port (e.g., 8443) will silently fall back to an insecure connection.
  • Conversely, if a caller legitimately runs plaintext gRPC on port 443, the code will forcibly apply TLS and fail at connection time.

A more robust approach would be to accept an explicit TLS flag or infer it from the scheme prefix (e.g., https://):

// Option 1: accept a bool from the caller
func NewClient(ctx context.Context, rpcAddress, grpcAddress, chainID string, tlsEnabled bool) (*Chain, error) { ... }

// Option 2: scheme-based detection
useTLS := strings.HasPrefix(grpcAddress, "https://") || strings.HasSuffix(grpcAddress, ":443")

At minimum, a comment explaining the convention would help future maintainers.

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@vladjdk vladjdk changed the title use tls on port 443 feat: use tls on port 443 Mar 19, 2026
@vladjdk vladjdk merged commit 2d4d9fd into main Mar 19, 2026
3 checks passed
@vladjdk vladjdk deleted the vlad/443-tls branch March 19, 2026 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants